On Tue, Apr 3, 2012 at 8:52 AM, Douglas Gregor <[email protected]> wrote: > > 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.
Possibly - I still don't have a clear intuition about how this is gauged. > 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 > '>>'? I'm not sure this is necessary, though. The previous tokens seem like it'd be relatively unambiguous that someone was trying to write a template template parameter regardless of further errors they may've written. I suppose if they wrote an extra ">" things could a little more confused, though. - David >> + 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
