[cfe-dev] ParseTag only called from ParseEnumSpecifier

[Ted Kremenek]

On Sep 3, 2008, at 12:12 PM, steve naroff wrote:

> Another solution is to have ParseClassSpecifier() call ParseTag().  
> At the moment, both of these routines call "ActOnTag" (which is a  
> little odd). If ParseClassSpecifier() called ParseTag(), the action  
> could be sent from one place. It also means we could set the  
> "TagKind" (Def, Decl, Ref) in one place. From my perspective,  
> centralizing both of these would be simplifying.
>
>> (2) It seems strange to me that parsing structs and unions, core
>> pieces of the C language, is done in ParseClassSpecifier
>> (ParseDeclCXX.cpp).  There seems to be a lot of commonality here that
>> could possibly be refactored and moved back into ParseDecl.cpp.  This
>> is my first look at this code in a while, so I'm don't claim to
>> understand all of the design decisions here, but conceptually it just
>> seems a little confusing.
>>
>
> Right...if ParseClassSpecifier() called ParseTag() it would make  
> more sense. I haven't looked that closely, however I imagine it  
> could be refactored to do this.
>
> Based on my previous comment, I guess I'd prefer we make ParseTag()  
> more reusable (rather than get rid of it).
>
> Thoughts?
>
> snaroff

These comments make a lot of sense to me.

Another possibility is that ParseTag calls ParseClassSpecifier.  That  
way the C++ specific parsing is delegated to by ParseTag, instead of  
having the commonality delegated to ParseTag by ParseClassSpecifier  
since the notion of a "class", at least as a "tag decl", is C++  
specific.

While we're doing these changes, it would also be nice if ActOnTag  
would have the information to distinguish between a forward  
declaration and a definition.  The current design *forces* the  
implementation of Actions to first construct a Decl object and then  
fill in its implementation (e.g., set the fields of a struct).  While  
in some implementations of the parser Actions this might be the best  
thing to do, forcing clients of Parse to do it this way just doesn't  
feel right to me.