[cfe-commits] r58913 - in /cfe/trunk: Driver/PrintParserCallbacks.cpp include/clang/Basic/DiagnosticKinds.def include/clang/Parse/Action.h include/clang/Parse/DeclSpec.h include/clang/Parse/Parser.h lib/Parse/MinimalAction.cpp lib/Parse/ParseDecl.cpp lib/Parse/ParseDeclCXX.cpp lib/Parse/ParseExpr.cpp lib/Parse/ParseExprCXX.cpp lib/Parse/ParseTentative.cpp lib/Parse/Parser.cpp lib/Sema/Sema.h lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExpr.cpp test/Lexer/cxx0x_keyword.cpp

[Argiris Kirtzidis]

Hi Doug,

Douglas Gregor wrote:
>
>> +  /// isTokenUnqualifiedId - True if token is the start of C++ 
>> unqualified-id
>> +  /// or an identifier in C.
>> +  ///
>> +  ///       unqualified-id:
>> +  ///         identifier
>> +  /// [C++]   operator-function-id
>> +  /// [C++]   conversion-function-id
>> +  /// [C++]   '~' class-name
>> +  /// [C++]   template-id      [TODO]
>> +  ///
>> +  bool isTokenUnqualifiedId() const {
>> +    return Tok.is(tok::identifier)  ||   // identifier or template-id
>> +           Tok.is(tok::kw_operator) ||   // 
>> operator/conversion-function-id or
>> +                                         // template-id
>> +           (Tok.is(tok::tilde) && getLang().CPlusPlus); // '~' 
>> class-name
>> +  }
>
> This works because a qualified-id will already have been annotated, so 
> the token we see would be a annot_cxxscope or annot_qualtype, right?

Yes, but isTokenUnqualifiedId was supposed to be used at 
ParseDirectDeclarator but after your changes in that function I didn't 
make use of it. I'll remove it, if there's a need for it we can bring it 
in again.

>
>> +      // If the next token is the name of the class type that the 
>> C++ scope
>> +      // denotes, followed by a '(', then this is a constructor 
>> declaration.
>> +      // We're done with the decl-specifiers.
>> +      if (Actions.isCurrentClassName(*NextToken().getIdentifierInfo(),
>> +                                     CurScope, &SS) &&
>> +          GetLookAheadToken(2).is(tok::l_paren))
>> +        goto DoneWithDeclSpec;
>>
>
> This won't parse constructors like
>
>     ((C::C))(int, float) {
>     }
>
> properly, right? I think the '&& 
> GetLookAheadToken(2).is(tok::l_paren)' needs to be removed.

A '(' means parsing of declaration specifiers is finished, 
Parser::ParseDeclarationSpecifiers will exit and this part of the code 
will not be reached.
You are doing a similar thing under "case tok::identifier":

      // C++: If the identifier is actually the name of the class type
      // being defined and the next token is a '(', then this is a
      // constructor declaration. We're done with the decl-specifiers
      // and will treat this token as an identifier.
      if (getLang().CPlusPlus &&
          CurScope->isCXXClassScope() &&
          Actions.isCurrentClassName(*Tok.getIdentifierInfo(), CurScope) &&
          NextToken().getKind() == tok::l_paren)
        goto DoneWithDeclSpec;


As a sidenote, "C::C" is messy stuff, since whether it's the constructor 
or the injected class-name depends on the context (ambiguous syntax in 
C++ ? what a surprise..)
I didn't take a crack at it yet, so out-of-line constructors are 
currently broken.

>
>> -bool Parser::isTypeSpecifierQualifier() const {
>> +bool Parser::isTypeSpecifierQualifier() {
>> +  // Annotate typenames and C++ scope specifiers.
>> +  TryAnnotateTypeOrScopeToken();
>> +
>>   switch (Tok.getKind()) {
>>   default: return false;
>>     // GNU attributes support.
>> @@ -1092,21 +1153,23 @@
>>   case tok::kw_const:
>>   case tok::kw_volatile:
>>   case tok::kw_restrict:
>> +
>> +    // typedef-name
>> +  case tok::annot_qualtypename:
>>     return true;
>>
>>     // GNU ObjC bizarre protocol extension: <proto1,proto2> with 
>> implicit 'id'.
>>   case tok::less:
>>     return getLang().ObjC1;
>> -
>> -    // typedef-name
>> -  case tok::identifier:
>> -    return Actions.isTypeName(*Tok.getIdentifierInfo(), CurScope) != 0;
>>   }
>> }
>
> Interesting. So we'll no longer see identifiers for typedef-names, 
> because they'll all be resolved to annot_qualtypename tokens?

Yes.

>
> In this case, should we be concerned that 
> TryAnnotateTypeOrScopeToken() will turn an identifier into a type, 
> when that identifier was actually meant to, e.g., introduce a new 
> name? I see that you avoided this issue within the declarator parsing 
> code (with TryAnnotateScopeToken), so I guess we'll just do that same 
> thing wherever there might be an issue.

Yes, exactly.

>
>>
>> +///         conversion-function-id                [TODO]
>
> And we don't handle this because... ah, because even though we can 
> parse a conversion-function-id with ParseConversionFunctionId, we 
> don't have a way to do name lookup on it yet. Looks like that's my 
> problem to deal with :)
>
>> +///         '~' class-name                        [TODO]
>
> Same issue here; this ball's in my court.
>
> Thanks! Moving on to the Sema bits now...

Thanks for reviewing!

-Argiris