Ping! On Fri, October 7, 2011 03:50, 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! > > Thanks, > Richard
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
