On Sat, Apr 7, 2012 at 10:30 AM, David Blaikie <[email protected]> wrote: > On Sat, Apr 7, 2012 at 12:52 AM, Francois Pichet <[email protected]> wrote: >> On Fri, Apr 6, 2012 at 1:26 AM, David Blaikie <[email protected]> wrote: >>> Author: dblaikie >>> Date: Fri Apr 6 00:26:43 2012 >>> New Revision: 154163 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=154163&view=rev >>> Log: >>> Restrict fixit for missing 'class' in template template parameters. >>> >>> Based on Doug's feedback to r153887 this omits the FixIt if the following >>> token >>> isn't syntactically valid for the context. (not a comma, '...', identifier, >>> '>', or '>>') >>> >>> There's a bunch of work to handle the '>>' case, but it makes for a much >>> more >>> pleasant diagnostic in this case. >>> >>> Added: >>> cfe/trunk/test/FixIt/no-fixit.cpp >>> Modified: >>> cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td >>> cfe/trunk/lib/Parse/ParseTemplate.cpp >>> cfe/trunk/test/FixIt/fixit-cxx0x.cpp >>> cfe/trunk/test/FixIt/fixit.cpp >>> cfe/trunk/test/Parser/cxx-template-decl.cpp >>> >>> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=154163&r1=154162&r2=154163&view=diff >>> ============================================================================== >>> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original) >>> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Fri Apr 6 >>> 00:26:43 2012 >>> @@ -479,8 +479,8 @@ >>> "unknown template name %0">; >>> def err_expected_comma_greater : Error< >>> "expected ',' or '>' in template-parameter-list">; >>> -def err_expected_class_on_template_template_param : Error< >>> - "template template parameters require 'class' after the argument list">; >>> +def err_class_on_template_template_param : Error< >>> + "template template parameter requires 'class' after the parameter list">; >>> def err_template_spec_syntax_non_template : Error< >>> "identifier followed by '<' indicates a class template specialization but >>> " >>> "%0 %select{does not refer to a template|refers to a function " >>> >>> Modified: cfe/trunk/lib/Parse/ParseTemplate.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTemplate.cpp?rev=154163&r1=154162&r2=154163&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/Parse/ParseTemplate.cpp (original) >>> +++ cfe/trunk/lib/Parse/ParseTemplate.cpp Fri Apr 6 00:26:43 2012 >>> @@ -312,11 +312,15 @@ >>> if (Tok.is(tok::greater)) >>> RAngleLoc = ConsumeToken(); >>> else if (ParseTemplateParameterList(Depth, TemplateParams)) { >>> - if (!Tok.is(tok::greater)) { >>> + if (Tok.is(tok::greatergreater)) { >>> + Tok.setKind(tok::greater); >>> + Tok.setLocation(Tok.getLocation().getLocWithOffset(1)); >>> + } else if (Tok.is(tok::greater)) >>> + RAngleLoc = ConsumeToken(); >>> + else { >>> Diag(Tok.getLocation(), diag::err_expected_greater); >>> return true; >>> } >>> - RAngleLoc = ConsumeToken(); >>> } >>> return false; >>> } >>> @@ -339,13 +343,13 @@ >>> } else { >>> // If we failed to parse a template parameter, skip until we find >>> // a comma or closing brace. >>> - SkipUntil(tok::comma, tok::greater, true, true); >>> + SkipUntil(tok::comma, tok::greater, tok::greatergreater, true, true); >> >> Not sure what you are trying to do here but this give the highly >> suspicious MSVC warning: >> >> 107>ParseTemplate.cpp(346): warning C4305: 'argument' : truncation >> from 'clang::tok::TokenKind' to 'bool' >> >> Basically the third argument 'tok::greatergreater' is for a bool >> param; will always be true. Is this the intend? > > Not at all intended. Seems I need more test cases for those failure > paths because they all suffer from the same problem (that SkipUntil > has convenience versions for one and two arguments - but not for more > (beyond two you need to use the array version)). Perhaps I'l add a > convenience version for 3 (& might fix up the array version to use > ArrayRef).
Fixed (along with some other things I found with my diagnostic improvements and by inspection) with test cases (& ArrayRef improvement) in r154325. > I'll also check whether this would've been caught by my improvements > to the implicit conversion warnings we have (I sent it for CR a few > days ago - by coincidence). Yep - the -Wconstant-conversion improvements caught this too. Yay. Thanks again, - David _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
