On Apr 2, 2012, at 12:15 PM, David Blaikie wrote: > Author: dblaikie > Date: Mon Apr 2 14:15:28 2012 > New Revision: 153887 > > URL: http://llvm.org/viewvc/llvm-project?rev=153887&view=rev > Log: > Correct error recovery when missing 'class' in a template template parameter. > > The diagnostic message correctly informs the user that they have omitted the > 'class' keyword, but neither suggests this insertion as a fixit, nor attempts > to recover as if they had provided the keyword. > > This fixes the recovery, adds the fixit, and adds a separate diagnostic and > corresponding replacement fixit for cases where the user wrote 'struct' or > 'typename' instead of 'class' (suggested by Richard Smith as a possible common > mistake). > > I'm not sure the diagnostic message for either the original or new cases feel > very Clang-esque, so I'm open to suggestions there. The fixit hints make it > fairly easy to see what's required, though. > > Modified: > cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td > cfe/trunk/lib/Parse/ParseTemplate.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=153887&r1=153886&r2=153887&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Mon Apr 2 14:15:28 > 2012 > @@ -480,6 +480,7 @@ > def err_expected_comma_greater : Error< > "expected ',' or '>' in template-parameter-list">; > def err_expected_class_before : Error<"expected 'class' before '%0'">; > +def err_expected_class_instead : Error<"expected 'class' instead of '%0'">; > 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=153887&r1=153886&r2=153887&view=diff > ============================================================================== > --- cfe/trunk/lib/Parse/ParseTemplate.cpp (original) > +++ cfe/trunk/lib/Parse/ParseTemplate.cpp Mon Apr 2 14:15:28 2012 > @@ -540,12 +540,17 @@ > > // Generate a meaningful error if the user forgot to put class before the > // identifier, comma, or greater. > - if (!Tok.is(tok::kw_class)) { > - Diag(Tok.getLocation(), diag::err_expected_class_before) > - << PP.getSpelling(Tok); > - return 0; > - } > - ConsumeToken(); > + if (Tok.is(tok::kw_typename) || Tok.is(tok::kw_struct)) { > + Diag(Tok.getLocation(), diag::err_expected_class_instead) > + << PP.getSpelling(Tok) > + << FixItHint::CreateReplacement(Tok.getLocation(), "class"); > + ConsumeToken(); > + } else if (!Tok.is(tok::kw_class)) > + Diag(Tok.getLocation(), diag::err_expected_class_before) > + << PP.getSpelling(Tok) > + << FixItHint::CreateInsertion(Tok.getLocation(), "class ");
This seems overly eager in providing a Fix-It. Shouldn't we check that the next token is something that actually makes sense for a template template parameter, e.g., '…', an identifier, a ',', '>', or '>>'? > + else > + ConsumeToken(); > > // Parse the ellipsis, if given. > SourceLocation EllipsisLoc; > > Modified: cfe/trunk/test/FixIt/fixit.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/fixit.cpp?rev=153887&r1=153886&r2=153887&view=diff > ============================================================================== > --- cfe/trunk/test/FixIt/fixit.cpp (original) > +++ cfe/trunk/test/FixIt/fixit.cpp Mon Apr 2 14:15:28 2012 > @@ -199,3 +199,8 @@ > expected-error {{missing 'typename' prior to dependent}} > return Mystery<T>::get(); > } > + > +template<template<typename> Foo, // expected-error {{expected 'class' before > 'Foo'}} > + template<typename> typename Bar, // expected-error {{expected > 'class' instead of 'typename'}} > + template<typename> struct Baz> // expected-error {{expected 'class' > instead of 'struct'}} > +void func(); > > Modified: cfe/trunk/test/Parser/cxx-template-decl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx-template-decl.cpp?rev=153887&r1=153886&r2=153887&view=diff > ============================================================================== > --- cfe/trunk/test/Parser/cxx-template-decl.cpp (original) > +++ cfe/trunk/test/Parser/cxx-template-decl.cpp Mon Apr 2 14:15:28 2012 > @@ -11,10 +11,8 @@ > // expected-warning {{declaration does not declare anything}} > template <template X> struct Err1; // expected-error {{expected '<' after > 'template'}} \ > // expected-error{{extraneous}} > -template <template <typename> > struct Err2; // expected-error > {{expected 'class' before '>'}} \ > -// expected-error{{extraneous}} > -template <template <typename> Foo> struct Err3; // expected-error > {{expected 'class' before 'Foo'}} \ > -// expected-error{{extraneous}} > +template <template <typename> > struct Err2; // expected-error > {{expected 'class' before '>'}} > +template <template <typename> Foo> struct Err3; // expected-error > {{expected 'class' before 'Foo'}} > > // Template function declarations > template <typename T> void foo(); > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
