On Apr 5, 2012, at 10:00 AM, David Blaikie <[email protected]> wrote:
> 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. We shouldn't emit the Fix-It unless it looks like we actually do have a template template parameter, which means checking that the next token is something that makes sense for a template template parameter following the 'class'. - Doug _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
