[cfe-dev] EnumConstantDecl is added to EnumDecl twice

[Douglas Gregor]

On Jan 7, 2009, at 7:26 PM, Zhongxing Xu wrote:

> 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 :)

	- Doug