<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jun 21, 2013 at 1:40 PM, Larisse Voufo <span dir="ltr"><<a href="mailto:lvoufo@google.com" target="_blank">lvoufo@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Ok. I'll take a look.<span class=""><font color="#888888"><div>
-- Larisse.</div><div><br></div></font></span></div><div class=""><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jun 21, 2013 at 1:33 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div>On Thu, Jun 20, 2013 at 5:08 PM, Larisse Voufo <<a href="mailto:lvoufo@google.com" target="_blank">lvoufo@google.com</a>> wrote:<br>
> Author: lvoufo<br>
> Date: Thu Jun 20 19:08:46 2013<br>
> New Revision: 184503<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=184503&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=184503&view=rev</a><br>
> Log:<br>
> Bug Fix: Template explicit instantiations should not have definitions (FixIts yet to be tested.)<br>
><br>
> Added:<br>
> cfe/trunk/test/CXX/temp/temp.spec/no-body.cpp<br>
> Modified:<br>
> cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td<br>
> cfe/trunk/lib/Parse/ParseDecl.cpp<br>
> cfe/trunk/lib/Parse/ParseDeclCXX.cpp<br>
> cfe/trunk/lib/Parse/ParseTemplate.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=184503&r1=184502&r2=184503&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=184503&r1=184502&r2=184503&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)<br>
> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Thu Jun 20 19:08:46 2013<br>
> @@ -592,6 +592,9 @@ def err_explicit_instantiation_with_defi<br>
> "explicit template instantiation cannot have a definition; if this "<br>
> "definition is meant to be an explicit specialization, add '<>' after the "<br>
> "'template' keyword">;<br>
> +def err_template_defn_explicit_instantiation : Error<<br>
> + "%select{function|class}0 cannot be defined in an explicit instantiation; if this "<br>
> + "declaration is meant to be a %select{function|class}0 definition, remove the 'template' keyword">;<br>
> def err_explicit_instantiation_enum : Error<<br>
> "enumerations cannot be explicitly instantiated">;<br>
> def err_expected_template_parameter : Error<"expected template parameter">;<br>
><br>
> Modified: cfe/trunk/lib/Parse/ParseDecl.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=184503&r1=184502&r2=184503&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=184503&r1=184502&r2=184503&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Parse/ParseDecl.cpp (original)<br>
> +++ cfe/trunk/lib/Parse/ParseDecl.cpp Thu Jun 20 19:08:46 2013<br>
> @@ -13,6 +13,7 @@<br>
><br>
> #include "clang/Parse/Parser.h"<br>
> #include "RAIIObjectsForParser.h"<br>
> +#include "clang/AST/DeclTemplate.h"<br>
> #include "clang/Basic/AddressSpaces.h"<br>
> #include "clang/Basic/CharInfo.h"<br>
> #include "clang/Basic/OpenCL.h"<br>
><br>
> Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=184503&r1=184502&r2=184503&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=184503&r1=184502&r2=184503&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)<br>
> +++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Thu Jun 20 19:08:46 2013<br>
> @@ -1539,6 +1539,14 @@ void Parser::ParseClassSpecifier(tok::To<br>
> } else {<br>
<br>
</div></div>The previous case in the if/else if chain seems to do the wrong thing<br>
in the presence of explicit instantiations too. This code triggers an<br>
assert (with or without your patch):<br>
<br>
template<typename T> struct A {};<br>
struct B { template friend struct A<int> {}; };<br>
<br></blockquote></div></div></div></div></blockquote><div style>Judging from other comments in the implementation of ParseClassSpecifier(), </div><div style>I find it very likely that the assert is wrongly specified -- a typo. </div>
<div style>It should allow TUK == TUK_Friend.</div><div style><br></div><div style>A broader and perhaps orthogonal question to answer here is whether all </div><div style>ill-formed friend declarations with template headers should be diagnosed in </div>
<div style>ActOnClassSpecialization() as is currently the case.</div><div style><br></div><div style>After fixing the typo, the above program produces the following diagnosis,</div><div style>treating the ill-formed friend declaration as likely a template specialization.</div>
<div style><pre style="color:rgb(0,0,0)">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.</pre></div><div><br></div><div style>I think that we should treat such friend declarations that only have the 'template' keyword </div><div style>as template header, as non-template friend declarations. See r184634.</div>
<div style>I modified the code slightly to produce the following diagnosis instead.</div><div style><pre style="color:rgb(0,0,0)">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.</pre></div><div style><br></div><div style>Please let me know if you don't like this so I can change it asap. But, at least for now there is </div><div style>no more runtime assertion.</div><div style>
<br></div><div style>Thanks,</div><div style>-- Larisse.</div><div style> </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><div class="h5"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
If you're not interested in fixing that, let me know and I'll file a bug.<br>
<div><div><br>
> if (TUK != Sema::TUK_Declaration && TUK != Sema::TUK_Definition)<br>
> ProhibitAttributes(attrs);<br>
> +<br>
> + if (TUK == Sema::TUK_Definition &&<br>
> + TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation) {<br>
> + // If the declarator-id is not a template-id, issue a diagnostic and<br>
> + // recover by ignoring the 'template' keyword.<br>
> + Diag(Tok, diag::err_template_defn_explicit_instantiation)<br>
> + << 1 << FixItHint::CreateRemoval(TemplateInfo.TemplateLoc);<br>
> + }<br>
><br>
> bool IsDependent = false;<br>
><br>
><br>
> Modified: cfe/trunk/lib/Parse/ParseTemplate.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTemplate.cpp?rev=184503&r1=184502&r2=184503&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTemplate.cpp?rev=184503&r1=184502&r2=184503&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Parse/ParseTemplate.cpp (original)<br>
> +++ cfe/trunk/lib/Parse/ParseTemplate.cpp Thu Jun 20 19:08:46 2013<br>
> @@ -236,8 +236,39 @@ Parser::ParseSingleDeclarationAfterTempl<br>
> << FixItHint::CreateRemoval(DS.getStorageClassSpecLoc());<br>
> DS.ClearStorageClassSpecs();<br>
> }<br>
> +<br>
> + if (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation) {<br>
> + if (DeclaratorInfo.getName().getKind() != UnqualifiedId::IK_TemplateId) {<br>
> + // If the declarator-id is not a template-id, issue a diagnostic and<br>
> + // recover by ignoring the 'template' keyword.<br>
> + Diag(Tok, diag::err_template_defn_explicit_instantiation) << 0;<br>
<br>
</div></div>Please add<br>
<br>
TemplateInfo = ParsedTemplateInfo();<br>
<br>
so that the downstream code sees a consistent state (this doesn't make<br>
any difference right now but should be more robust against future<br>
changes).<br>
<div><br>
> + } else {<br>
> + SourceLocation LAngleLoc<br>
> + = PP.getLocForEndOfToken(TemplateInfo.TemplateLoc);<br>
> + Diag(DeclaratorInfo.getIdentifierLoc(),<br>
> + diag::err_explicit_instantiation_with_definition)<br>
> + << SourceRange(TemplateInfo.TemplateLoc)<br>
> + << FixItHint::CreateInsertion(LAngleLoc, "<>");<br>
> +<br>
> + // Recover as if it were an explicit specialization.<br>
> + TemplateParameterLists ParamLists;<br>
> + SmallVector<Decl*, 4> TemplateParams;<br>
> + ParamLists.push_back(<br>
> + TemplateParameterList::Create(Actions.getASTContext(),<br>
> + TemplateInfo.TemplateLoc,<br>
> + LAngleLoc,<br>
> + (NamedDecl**)TemplateParams.data(),<br>
> + TemplateParams.size(), LAngleLoc));<br>
<br>
</div>You can use "0, 0," instead of TemplateParams.data() and<br>
TemplateParams.size() here, and remove the TemplateParams variable.<br>
<br>
Also, please use Actions.ActOnTemplateParameterList, rather than<br>
creating the template parameter list directly. We generally avoid<br>
directly manipulating the AST from the Parser.<br>
<div><div><br>
> +<br>
> + return ParseFunctionDefinition(DeclaratorInfo,<br>
> + ParsedTemplateInfo(&ParamLists,<br>
> + /*isSpecialization=*/true,<br>
> + /*LastParamListWasEmpty=*/true),<br>
> + &LateParsedAttrs);<br>
> + }<br>
> + }<br>
> return ParseFunctionDefinition(DeclaratorInfo, TemplateInfo,<br>
> - &LateParsedAttrs);<br>
> + &LateParsedAttrs);<br>
> }<br>
><br>
> // Parse this declaration.<br>
><br>
> Added: cfe/trunk/test/CXX/temp/temp.spec/no-body.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/temp/temp.spec/no-body.cpp?rev=184503&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/temp/temp.spec/no-body.cpp?rev=184503&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/CXX/temp/temp.spec/no-body.cpp (added)<br>
> +++ cfe/trunk/test/CXX/temp/temp.spec/no-body.cpp Thu Jun 20 19:08:46 2013<br>
> @@ -0,0 +1,52 @@<br>
> +// RUN: %clang_cc1 -fsyntax-only -verify %s<br>
> +<br>
> +template<typename T> void f(T) { }<br>
> +template<typename T> void g(T) { }<br>
> +template<typename T> struct x { };<br>
> +template<typename T> struct y { }; // expected-note {{declared here}}<br>
> +<br>
> +namespace good {<br>
> + template void f<int>(int);<br>
> + template void g(int);<br>
> + template struct x<int>;<br>
> +}<br>
> +<br>
> +namespace unsupported {<br>
> + template struct y; // expected-error {{elaborated type refers to a template}}<br>
> +}<br>
> +<br>
> +template<typename T> void f0(T) { }<br>
> +template<typename T> void g0(T) { }<br>
> +template<typename T> struct x0 { }; // expected-note {{explicitly specialized declaration is here}}<br>
> +template<typename T> struct y0 { };<br>
> +<br>
> +// Should recover as if definition<br>
> +namespace noargs_body {<br>
> + 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}}<br>
> + 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}}<br>
> +}<br>
> +<br>
> +// Explicit specializations expected in global scope<br>
> +namespace exp_spec {<br>
> + template<> void f0<int>(int) { } // expected-error {{no function template matches function template specialization 'f0'}}<br>
> + template<> struct x0<int> { }; // expected-error {{class template specialization of 'x0' must originally be declared in the global scope}}<br>
> +}<br>
> +<br>
> +template<typename T> void f1(T) { }<br>
> +template<typename T> struct x1 { }; // expected-note {{explicitly specialized declaration is here}}<br>
> +<br>
> +// Should recover as if specializations,<br>
> +// thus also complain about not being in global scope.<br>
> +namespace args_bad {<br>
> + 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}} \<br>
> + expected-error {{no function template matches function template specialization 'f1'}}<br>
> + 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}} \<br>
> + expected-error {{class template specialization of 'x1' must originally be declared in the global scope}}<br>
> +}<br>
> +<br>
> +template<typename T> void f2(T) { }<br>
> +template<typename T> struct x2 { };<br>
> +<br>
> +// Should recover as if specializations<br>
> +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}}<br>
> +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}}<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>