[cfe-dev] ParseTag only called from ParseEnumSpecifier

[steve naroff]

Doug,

Thanks a lot for chiming in...your comments make a lot of sense.

I guess if we want clang to have a unified parser for C/ObjC/C++, it's  
inevitable that some aspects of the parser will start feeling less C- 
like (particularly for structs/classes...). As you say, refining the  
comments may help. We could also consider coming up with a better name  
for the action hook (ActOnTag), however that's purely aesthetic.

snaroff


On Sep 4, 2008, at 2:42 AM, Doug Gregor wrote:

> On Wed, Sep 3, 2008 at 2:36 PM, Ted Kremenek <kremenek at apple.com>  
> wrote:
>> I noticed today that ParseTag is only called by ParseEnumSpecifier.
>> In the past ParseTag was used do much of the heavy lifting of parsing
>> structs, unions, and enums, and the comments on the source still say
>> this.  Now it looks like ParseTag is only called by
>> ParseEnumSpecifier, which means most of the comments are not true and
>> that the logic could probably just be rolled into ParseEnumSpecifier
>> directly.
>
> I think I made this change, so I'll chime in.
>
>> From my understanding of the code, it appears that
>> ParseClassSpecifier (in ParseDeclCXX.cpp) now does all the parsing  
>> for
>> structs, unions, and C++ classes.  This seems a little weird to me,
>> and it isn't clear from the comments in the Parse library that this  
>> is
>> the case.
>
> That comment in ParseClassSpecifier is very C++-standards-wonk; I'll
> be happy to make that clearer.
>
>> Specifically, I have two concerns:
>>
>> (1) ParseTag now appears to be somewhat redundant and unnecessary,  
>> and
>> in some ways ill-named.  It doesn't appear to serve its original
>> purpose anymore, and thus it's logic should probably just be folded
>> into ParseEnumSpecifier.
>
> I agree.
>
>> (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.
>
> The C++ class-specifier (with elaborated-type-specifier) parses a
> superset of the C struct-or-union-specifier, and in C mode
> ParseClassSpecifier falls back to parsing struct-or-union-specifier.
> It didn't make sense to me to put ParseClassSpecifier into the C
> ParseDecl.cpp, because class-specifiers aren't a C concept.
>
> While it's possible to make ParseClassSpecifier delegate to ParseTag,
> I don't think it's the right approach for the future. ParseTag (which
> parses the C concept of tags) would have to deal with some nontrivial
> C++-specific parsing details for, e.g.,
>
>  class X<int>::Y
>
> Clang isn't parsing nested-name-specifiers or template-ids, so the
> ParseClassSpecifier/ParseTag code looks a lot more similar now than it
> would look if Clang had support for nested classes or class template
> specializations. ParseTag could be made to deal with these C++-isms,
> but I don't think it should be: let the non-trivial C++ parsing stay
> in the C++ part of the parser.
>
> Also, while the C99 struct-or-union-specifier and enum-specifier
> nonterminals are similar enough to permit implementation with a single
> ParseTag function, the C++ equivalents (class-specifier,
> enum-specifier and elaborated-type-specifier) have more divergent
> grammars, so it makes sense to keep ParseClassSpecifier and
> ParseEnumSpecifier separate for C++. As more of C++ (and C++0x) gets
> implemented, the common code between these two will be a much smaller
> proportion of the actual parsing functions.
>
>> As a side note, my original interest in this code was to allow the
>> parser+actions to be able to distinguish up front between enum/ 
>> struct/
>> union/class forward declarations and definitions.
>
> FWIW, this requires arbitrary lookahead in C++ (but not in C), since
> we can't tell the difference between a class-specifier and an
> elaborated-type-specifier until we've parsed through the
> nested-name-specifier and hit the actual class identifier (or
> template-id). Of course, one could apply Argiris' pre-parser idea to
> do the disambiguation between these two nonterminals in advance.
>
>  - Doug
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev