[cfe-dev] ParseTag only called from ParseEnumSpecifier
doug.gregor at gmail.com
Wed Sep 3 23:42:11 PDT 2008
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
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.
> (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.,
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.
More information about the cfe-dev