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

[Eli Friedman]

On Wed, Oct 5, 2011 at 5:25 PM, Richard Smith <richard at metafoo.co.uk> 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. Also, MightBeDeclarator intentionally allows some
> non-declarator cases which are probable typos for declarators. In any case,
> TryParseDeclarator succeeds on the example above, parsing up to the '.'.
>
> What are your cases? Even if we change approach, I'd like to add tests for them.

Err, hmm, one of them wasn't actually right.  I'm pretty sure the
following is valid, though:

int x, y alignas(float);

-Eli