On Wed, October 19, 2011 06:24, Douglas Gregor wrote: > On Oct 6, 2011, at 7:50 PM, Richard Smith wrote: >>> 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 <[email protected]> >>>>> 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. >> >> On Thu, October 6, 2011 01:44, John McCall wrote: >> >>> Yes, please don't use tentative parsing here. :) In fact, if you could >>> use at most one token of lookahead, that would be ideal. >> >> Indeed, that is how the patch operates. >> >> >>> 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. >>> >> >> I'm not hugely enamored with the approach either, but I think a black-list >> of "obvious" non-declarators would be worse. I've added a comment as you >> requested. >> >> On Thu, October 6, 2011 01:59, Eli Friedman wrote: >> >>> On Wed, Oct 5, 2011 at 5:25 PM, Richard Smith <[email protected]> >>> wrote: >>> >>>> 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); >> >> Thanks! I'd accidentally added kw_alignas to the 'before identifier' list >> rather than the 'after identifier' list where it belongs. >> >> The attached patch fixes that, adds support for code_completion tokens, and >> adds a few more cases to the whitelist where the existing error recovery >> was better than the new error recovery (for instance, in "{ int a, \n b }"). >> I've >> gone over the declaration parsing code again to check I didn't miss any >> tokens, and tested this against a very large quantity of C++ code with no >> rejects-valids. Further comments welcome! > > This LGTM. Clever.
Thanks, in r142544. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
