[cfe-commits] r153887 - in /cfe/trunk: include/clang/Basic/DiagnosticParseKinds.td lib/Parse/ParseTemplate.cpp test/FixIt/fixit.cpp test/Parser/cxx-template-decl.cpp

[David Blaikie]

On Tue, Apr 3, 2012 at 8:52 AM, Douglas Gregor <dgregor at apple.com> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>