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
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
(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.