[cfe-dev] C++ PATCH: Basic inheritance syntax, semantics
clattner at apple.com
Sun Apr 13 16:57:29 CDT 2008
On Apr 13, 2008, at 2:02 PM, Doug Gregor wrote:
>> Also, perhaps ParseStructUnionSpecifier/ParseClassSpecifier
>> should be renamed ParseStructUnionClassSpecifier, though that's
>> pretty long
>> :). ParseSUCSpecifier?
> Yuck. In C++-speak, it's just a "class specifier" because classes,
> structs, and unions are all class types. That's why I prefer the name
> ParseClassSpecifier (but the comment is helpful).
Ok, fair enough!
>> + AccessSpecifier IsAccessSpecifier();
>> should be 'const'. Also, for better or worse, other 'is'
>> predicates in
>> Parser (e.g. isDeclarationSpecifier) use a lowercase 'i', please be
>> consistent with that. Also, it's somewhat strange for an 'is'
>> method to
>> return something other than bool. Maybe this should be:
>> "getAccessSpecifierIfPresent()" or something like that?
> I like getAccessSpecifierIfPresent; let's go with that.
>> + // Find the complete source range for the base-specifier.
>> + // FIXME: Is there a better way to
>> + SourceRange Range(StartLoc);
>> + unsigned RawSpecifierEnd
>> + = Tok.getLocation().getRawEncoding() + Tok.getLength();
>> You shouldn't need to do this. The end location of a source range
>> points to the first character of the last token.
> ... and that token is still highlighted with the source range. I
> didn't realize that. Thanks!
np, it is very non-obvious, but is an important simplification for
clients as you noticed ;-)
> FWIW, the second patch I sent just added the regression test and fixed
> a typo or two. Nothing that requires a separate review.
Ok, sounds great!
More information about the cfe-dev