[cfe-commits] r110436 - in /cfe/trunk: include/clang/Basic/TokenKinds.def lib/Parse/ParseCXXInlineMethods.cpp lib/Parse/ParseDecl.cpp test/Parser/cxx-default-args.cpp

[Argyrios Kyrtzidis]

On Aug 6, 2010, at 11:13 AM, Douglas Gregor wrote:

> 
> On Aug 6, 2010, at 11:47 AM, Argyrios Kyrtzidis wrote:
> 
>> Author: akirtzidis
>> Date: Fri Aug  6 04:47:24 2010
>> New Revision: 110436
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=110436&view=rev
>> Log:
>> Introduce a new token kind 'cxx_defaultarg_end' to mark the end of C++ default arguments that were part of
>> lexed method declarations.
>> 
>> This avoid interference with tokens coming after the point where the default arg tokens were 'injected', e.g. for
>> 
>> typedef struct Inst {
>> void m(int x=0);
>> } *InstPtr;
>> 
>> when parsing '0' the next token would be '*' and things would be messed up.
> 
> Good idea.
> 
>> Modified:
>>   cfe/trunk/include/clang/Basic/TokenKinds.def
>>   cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
>>   cfe/trunk/lib/Parse/ParseDecl.cpp
>>   cfe/trunk/test/Parser/cxx-default-args.cpp
>> 
>> Modified: cfe/trunk/include/clang/Basic/TokenKinds.def
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TokenKinds.def?rev=110436&r1=110435&r2=110436&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/TokenKinds.def (original)
>> +++ cfe/trunk/include/clang/Basic/TokenKinds.def Fri Aug  6 04:47:24 2010
>> @@ -96,6 +96,7 @@
>> TOK(eof)                 // End of file.
>> TOK(eom)                 // End of macro (end of line inside a macro).
>> TOK(code_completion)     // Code completion marker
>> +TOK(cxx_defaultarg_end)  // C++ default argument end marker
>> 
>> // C99 6.4.9: Comments.
>> TOK(comment)             // Comment (only in -E -C[C] mode)
>> 
>> Modified: cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp?rev=110436&r1=110435&r2=110436&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp (original)
>> +++ cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp Fri Aug  6 04:47:24 2010
>> @@ -142,9 +142,13 @@
>>        OwningExprResult DefArgResult(ParseAssignmentExpression());
>>        if (DefArgResult.isInvalid())
>>          Actions.ActOnParamDefaultArgumentError(LM.DefaultArgs[I].Param);
>> -        else
>> +        else {
>> +          assert(Tok.is(tok::cxx_defaultarg_end) &&
>> +                 "We didn't parse the whole default arg!");
>> +          ConsumeToken(); // Consume tok::cxx_defaultarg_end.
> 
> How about we SkipUntil the tok::cxx_defaultarg_end, and give an error if it's not the next token? That way, if expression parsing doesn't use up all of the tokens, we'll give a reasonable diagnostic rather than asserting.

But if there was no error in expression parsing, clang should have used up all of the tokens, otherwise it's a bug; can DefArgResult be valid after expression parsing aborted due to an error ?
What kind of diagnostic do you have in mind, wouldn't it seem strange since the user's code would have no actual problem ?

-Argiris