EnumConstantDecl is added to EnumDecl twice

Hi,

Currently every EnumConstantDecl is added to its EnumDecl twice: one at SemaDecl.cpp:3489, the other at SemaDecl.cpp:109.

I noticed the comments said that the enumerator is stored in two DeclContexts. Maybe we should change line SemaDecl.cpp:109 :

CurContext->addDecl(Context, SD);

into:

((DeclContext *)S->getEntity())->addDecl(Context, SD);

to make it conform to the comments.

Hi,

Currently every EnumConstantDecl is added to its EnumDecl twice: one at SemaDecl.cpp:3489, the other at SemaDecl.cpp:109.

I noticed the comments said that the enumerator is stored in two DeclContexts. Maybe we should change line SemaDecl.cpp:109 :

CurContext->addDecl(Context, SD);

into:

((DeclContext *)S->getEntity())->addDecl(Context, SD);

That's definitely the right direction to go. If we can completely decouple PushOnScopeChains from CurContext, then it will be possible to PushOnScopeChains(Decl, AnyScopeYouWantToUse), which will be very helpful when it comes to, e.g., pushing implicitly-declared entities into the translation unit's scope. Perhaps we could even get rid of CurContext entirely, in favor of finding the context based on the Scope we're given?

Regarding that specific change: it isn't quite right, because the code just above it will skip through any transparent contexts, but it won't guarantee that S->getEntity() is non-NULL. From the original scope S we get, we'll need to find the first scope that has an entity (S- >getEntity() != NULL) and add the scoped declaration to that DeclContext. (That DeclContext itself might be a transparent DeclContext, like an enumeration's DeclContext).

Stepping back to the comment that the enumerator is stored in two DeclContexts: we should be able to remove the addDecl and setLexicalDeclContext calls after the FIXME near line 3489 of SemaDecl.cpp, because transparent DeclContexts should already cope with this case. I actually meant to make that change as part of the transparent DeclContext patch, but it didn't seem to happen :slight_smile:

  - Doug

FWIW, this code is gone now, and transparent declcontexts are handling name lookup for enumerators.

  - Doug