[cfe-commits] [PATCH] Improve error recovery for comma/semicolon typo in declaration group

[John McCall]

On Oct 5, 2011, at 5:25 PM, Richard Smith wrote:
> On Thu, October 6, 2011 00:58, Eli Friedman wrote:
>> On Wed, Oct 5, 2011 at 4:26 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>> Clang currently gives unhelpful diagnostics for cases such as this:
>>> 
>>> 
>>>      static const char *const Triples[] = {
>>>        "powerpc-linux-gnu",
>>>        "powerpc-unknown-linux-gnu"
>>>      },
>>>      CandidateLibDirs.append(LibDirs, LibDirs +
>>> llvm::array_lengthof(LibDirs));
>>> 
>>> 
>>> Viz:
>>> 
>>> 
>>> lib/Driver/ToolChains.cpp:1582:7: error: default initialization of an
>>> object of const type 'const char'     CandidateLibDirs.append(LibDirs,
>>> LibDirs + llvm::array_lengthof(LibDirs));
>>>     ^
>>> lib/Driver/ToolChains.cpp:1582:23: error: expected ';' at end of declaration
>>>     CandidateLibDirs.append(LibDirs, LibDirs +
>>> llvm::array_lengthof(LibDirs));
>>>                     ^
>>>                     ;
>>> 
>>> 
>>> The attached patch is a conservative fix for this issue. In cases where a
>>> declarator group contains a comma followed by a newline followed by
>>> something which obviously is neither a declarator nor a typo for a
>>> declarator, we give a fixit suggesting that a ; was intended:
>>> 
>>> lib/Driver/ToolChains.cpp:1581:8: error: expected ';' at end of declaration
>>>      },
>>>       ^ ;
>>> 
>>> 
>>> OK to commit?
>>> 
>> 
>> I don't really like Parser::MightBeDeclarator... I can see at least
>> two cases where it rejects valid code.  Can you use Parser::TryParseDeclarator
>> or something?
> 
> I'd be reluctant to add tentative parsing for all declarator groups with more
> than one declarator.

Yes, please don't use tentative parsing here. :)  In fact, if you could use at most
one token of lookahead, that would be ideal.

Abstractly, I don't really like the approach of opting-out possible declarators,
but the alternative would make the diagnostic *very* special-case.  Please
at least leave a comment in the declarator-parsing code saying to update
your function if a new token is added there, though.

John.