r184503 - Bug Fix: Template explicit instantiations should not have definitions (FixIts yet to be tested.)

Larisse Voufo lvoufo at google.com
Sat Jun 22 06:57:51 PDT 2013


On Fri, Jun 21, 2013 at 1:40 PM, Larisse Voufo <lvoufo at google.com> wrote:

> Ok. I'll take a look.
> -- Larisse.
>
>
>
> On Fri, Jun 21, 2013 at 1:33 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>
>> On Thu, Jun 20, 2013 at 5:08 PM, Larisse Voufo <lvoufo at google.com> wrote:
>> > Author: lvoufo
>> > Date: Thu Jun 20 19:08:46 2013
>> > New Revision: 184503
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=184503&view=rev
>> > Log:
>> > Bug Fix: Template explicit instantiations should not have definitions
>> (FixIts yet to be tested.)
>> >
>> > Added:
>> >     cfe/trunk/test/CXX/temp/temp.spec/no-body.cpp
>> > Modified:
>> >     cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>> >     cfe/trunk/lib/Parse/ParseDecl.cpp
>> >     cfe/trunk/lib/Parse/ParseDeclCXX.cpp
>> >     cfe/trunk/lib/Parse/ParseTemplate.cpp
>> >
>> > Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=184503&r1=184502&r2=184503&view=diff
>> >
>> ==============================================================================
>> > --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
>> > +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Thu Jun 20
>> 19:08:46 2013
>> > @@ -592,6 +592,9 @@ def err_explicit_instantiation_with_defi
>> >      "explicit template instantiation cannot have a definition; if this
>> "
>> >      "definition is meant to be an explicit specialization, add '<>'
>> after the "
>> >      "'template' keyword">;
>> > +def err_template_defn_explicit_instantiation : Error<
>> > +  "%select{function|class}0 cannot be defined in an explicit
>> instantiation; if this "
>> > +  "declaration is meant to be a %select{function|class}0 definition,
>> remove the 'template' keyword">;
>> >  def err_explicit_instantiation_enum : Error<
>> >      "enumerations cannot be explicitly instantiated">;
>> >  def err_expected_template_parameter : Error<"expected template
>> parameter">;
>> >
>> > Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=184503&r1=184502&r2=184503&view=diff
>> >
>> ==============================================================================
>> > --- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
>> > +++ cfe/trunk/lib/Parse/ParseDecl.cpp Thu Jun 20 19:08:46 2013
>> > @@ -13,6 +13,7 @@
>> >
>> >  #include "clang/Parse/Parser.h"
>> >  #include "RAIIObjectsForParser.h"
>> > +#include "clang/AST/DeclTemplate.h"
>> >  #include "clang/Basic/AddressSpaces.h"
>> >  #include "clang/Basic/CharInfo.h"
>> >  #include "clang/Basic/OpenCL.h"
>> >
>> > Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=184503&r1=184502&r2=184503&view=diff
>> >
>> ==============================================================================
>> > --- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
>> > +++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Thu Jun 20 19:08:46 2013
>> > @@ -1539,6 +1539,14 @@ void Parser::ParseClassSpecifier(tok::To
>> >    } else {
>>
>> The previous case in the if/else if chain seems to do the wrong thing
>> in the presence of explicit instantiations too. This code triggers an
>> assert (with or without your patch):
>>
>>   template<typename T> struct A {};
>>   struct B { template friend struct A<int> {}; };
>>
>> Judging from other comments in the implementation of
ParseClassSpecifier(),
I find it very likely that the assert is wrongly specified -- a typo.
It should allow TUK == TUK_Friend.

A broader and perhaps orthogonal question to answer here is whether all
ill-formed friend declarations with template headers should be diagnosed in
ActOnClassSpecialization() as is currently the case.

After fixing the typo, the above program produces the following diagnosis,
treating the ill-formed friend declaration as likely a template
specialization.

test.cpp:5:42: error: cannot define a type in a friend declaration
struct B { template friend struct A<int> {}; };
                    ~~~~~~               ^
test.cpp:5:35: error: explicit template instantiation cannot have a
definition; if
      this definition is meant to be an explicit specialization, add
'<>' after the 'template' keyword
struct B { template friend struct A<int> {}; };
           ~~~~~~~~               ^
                   <>
test.cpp:5:28: error: template specialization declaration cannot be a friend
struct B { template friend struct A<int> {}; };
           ~~~~~~~~~       ^       ~~~~~
3 errors generated.


I think that we should treat such friend declarations that only have the
'template' keyword
as template header, as non-template friend declarations. See r184634.
I modified the code slightly to produce the following diagnosis instead.

test.cpp:5:42: error: cannot define a type in a friend declaration
struct B { template friend struct A<int> {}; };
                    ~~~~~~               ^
test.cpp:5:21: error: friend cannot be declared in an explicit
instantiation; if this
      declaration is meant to be a friend declaration, remove the
'template' keyword
struct B { template friend struct A<int> {}; };
           ~~~~~~~~~^
2 errors generated.


Please let me know if you don't like this so I can change it asap. But, at
least for now there is
no more runtime assertion.

Thanks,
-- Larisse.


 If you're not interested in fixing that, let me know and I'll file a bug.
>>
>> >      if (TUK != Sema::TUK_Declaration && TUK != Sema::TUK_Definition)
>> >        ProhibitAttributes(attrs);
>> > +
>> > +    if (TUK == Sema::TUK_Definition &&
>> > +        TemplateInfo.Kind ==
>> ParsedTemplateInfo::ExplicitInstantiation) {
>> > +      // If the declarator-id is not a template-id, issue a diagnostic
>> and
>> > +      // recover by ignoring the 'template' keyword.
>> > +      Diag(Tok, diag::err_template_defn_explicit_instantiation)
>> > +        << 1 << FixItHint::CreateRemoval(TemplateInfo.TemplateLoc);
>> > +    }
>> >
>> >      bool IsDependent = false;
>> >
>> >
>> > Modified: cfe/trunk/lib/Parse/ParseTemplate.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTemplate.cpp?rev=184503&r1=184502&r2=184503&view=diff
>> >
>> ==============================================================================
>> > --- cfe/trunk/lib/Parse/ParseTemplate.cpp (original)
>> > +++ cfe/trunk/lib/Parse/ParseTemplate.cpp Thu Jun 20 19:08:46 2013
>> > @@ -236,8 +236,39 @@ Parser::ParseSingleDeclarationAfterTempl
>> >          << FixItHint::CreateRemoval(DS.getStorageClassSpecLoc());
>> >        DS.ClearStorageClassSpecs();
>> >      }
>> > +
>> > +    if (TemplateInfo.Kind ==
>> ParsedTemplateInfo::ExplicitInstantiation) {
>> > +      if (DeclaratorInfo.getName().getKind() !=
>> UnqualifiedId::IK_TemplateId) {
>> > +        // If the declarator-id is not a template-id, issue a
>> diagnostic and
>> > +        // recover by ignoring the 'template' keyword.
>> > +        Diag(Tok, diag::err_template_defn_explicit_instantiation) << 0;
>>
>> Please add
>>
>>   TemplateInfo = ParsedTemplateInfo();
>>
>> so that the downstream code sees a consistent state (this doesn't make
>> any difference right now but should be more robust against future
>> changes).
>>
>> > +      } else {
>> > +        SourceLocation LAngleLoc
>> > +          = PP.getLocForEndOfToken(TemplateInfo.TemplateLoc);
>> > +        Diag(DeclaratorInfo.getIdentifierLoc(),
>> > +             diag::err_explicit_instantiation_with_definition)
>> > +          << SourceRange(TemplateInfo.TemplateLoc)
>> > +          << FixItHint::CreateInsertion(LAngleLoc, "<>");
>> > +
>> > +        // Recover as if it were an explicit specialization.
>> > +        TemplateParameterLists ParamLists;
>> > +        SmallVector<Decl*, 4> TemplateParams;
>> > +        ParamLists.push_back(
>> > +            TemplateParameterList::Create(Actions.getASTContext(),
>> > +                                          TemplateInfo.TemplateLoc,
>> > +                                          LAngleLoc,
>> > +
>>  (NamedDecl**)TemplateParams.data(),
>> > +                                          TemplateParams.size(),
>> LAngleLoc));
>>
>> You can use "0, 0," instead of TemplateParams.data() and
>> TemplateParams.size() here, and remove the TemplateParams variable.
>>
>> Also, please use Actions.ActOnTemplateParameterList, rather than
>> creating the template parameter list directly. We generally avoid
>> directly manipulating the AST from the Parser.
>>
>> > +
>> > +        return ParseFunctionDefinition(DeclaratorInfo,
>> > +                                       ParsedTemplateInfo(&ParamLists,
>> > +                                           /*isSpecialization=*/true,
>> > +
>> /*LastParamListWasEmpty=*/true),
>> > +                                       &LateParsedAttrs);
>> > +      }
>> > +    }
>> >      return ParseFunctionDefinition(DeclaratorInfo, TemplateInfo,
>> > -                                   &LateParsedAttrs);
>> > +
>>  &LateParsedAttrs);
>> >    }
>> >
>> >    // Parse this declaration.
>> >
>> > Added: cfe/trunk/test/CXX/temp/temp.spec/no-body.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/temp/temp.spec/no-body.cpp?rev=184503&view=auto
>> >
>> ==============================================================================
>> > --- cfe/trunk/test/CXX/temp/temp.spec/no-body.cpp (added)
>> > +++ cfe/trunk/test/CXX/temp/temp.spec/no-body.cpp Thu Jun 20 19:08:46
>> 2013
>> > @@ -0,0 +1,52 @@
>> > +// RUN: %clang_cc1 -fsyntax-only -verify %s
>> > +
>> > +template<typename T> void f(T) { }
>> > +template<typename T> void g(T) { }
>> > +template<typename T> struct x { };
>> > +template<typename T> struct y { };  // expected-note {{declared here}}
>> > +
>> > +namespace good {
>> > +  template void f<int>(int);
>> > +  template void g(int);
>> > +  template struct x<int>;
>> > +}
>> > +
>> > +namespace unsupported {
>> > + template struct y;     // expected-error {{elaborated type refers to
>> a template}}
>> > +}
>> > +
>> > +template<typename T> void f0(T) { }
>> > +template<typename T> void g0(T) { }
>> > +template<typename T> struct x0 { }; // expected-note {{explicitly
>> specialized declaration is here}}
>> > +template<typename T> struct y0 { };
>> > +
>> > +// Should recover as if definition
>> > +namespace noargs_body {
>> > +  template void g0(int) { } // expected-error {{function cannot be
>> defined in an explicit instantiation; if this declaration is meant to be a
>> function definition, remove the 'template' keyword}}
>> > +  template struct y0 { };   // expected-error {{class cannot be
>> defined in an explicit instantiation; if this declaration is meant to be a
>> class definition, remove the 'template' keyword}}
>> > +}
>> > +
>> > +// Explicit specializations expected in global scope
>> > +namespace exp_spec {
>> > +  template<> void f0<int>(int) { }  // expected-error {{no function
>> template matches function template specialization 'f0'}}
>> > +  template<> struct x0<int> { };    // expected-error {{class template
>> specialization of 'x0' must originally be declared in the global scope}}
>> > +}
>> > +
>> > +template<typename T> void f1(T) { }
>> > +template<typename T> struct x1 { };  // expected-note {{explicitly
>> specialized declaration is here}}
>> > +
>> > +// Should recover as if specializations,
>> > +// thus also complain about not being in global scope.
>> > +namespace args_bad {
>> > +  template void f1<int>(int) { }    // expected-error {{explicit
>> template instantiation cannot have a definition; if this definition is
>> meant to be an explicit specialization, add '<>' after the 'template'
>> keyword}} \
>> > +                                       expected-error {{no function
>> template matches function template specialization 'f1'}}
>> > +  template struct x1<int> { };      // expected-error {{explicit
>> template instantiation cannot have a definition; if this definition is
>> meant to be an explicit specialization, add '<>' after the 'template'
>> keyword}} \
>> > +                                       expected-error {{class template
>> specialization of 'x1' must originally be declared in the global scope}}
>> > +}
>> > +
>> > +template<typename T> void f2(T) { }
>> > +template<typename T> struct x2 { };
>> > +
>> > +// Should recover as if specializations
>> > +template void f2<int>(int) { }    // expected-error {{explicit
>> template instantiation cannot have a definition; if this definition is
>> meant to be an explicit specialization, add '<>' after the 'template'
>> keyword}}
>> > +template struct x2<int> { };      // expected-error {{explicit
>> template instantiation cannot have a definition; if this definition is
>> meant to be an explicit specialization, add '<>' after the 'template'
>> keyword}}
>> >
>> >
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130622/81353eb2/attachment.html>


More information about the cfe-commits mailing list