[& of course I forget to include the patch. Attached now]
On Thu, Oct 13, 2011 at 10:09 PM, David Blaikie <[email protected]> wrote:
> From the bug:
>
> template<int ...Values, int After> struct X0nt;
> X0nt<42> f();
>
>
> Causes clang to fail an assertion (SemaTemplate.cpp:4868 - "Converted
> template argument list is too short!"). This fix causes
> CheckTemplateArgumentList to fail if there are parameter packs anywhere
> other than the last argument in a non-partial application of the template
> arguments (partial applications include function templates for which
> non-final parameter packs may be valid. This change does not regress that
> behavior as far as I know/as far as the test cases already cover it).
>
> I've included a test for this fix.
>
> As an added bonus, I've also fixed a minor quirk in the output of the
> diagnostic that appears here ("template parameter pack must be the last
> template parameter") - currently/without my patch, Clang produces this
> diagnostic for every parameter after a parameter pack. So in the case of
> "template<typename ...A, typename B, typename C> struct foo;" the error is
> emitted twice (though, strangely, the second type it is produced it does not
> provide the context/line location, though the line/col numbers are the same
> & still correct). I've fixed that issue & modified a test case to catch it
> too.
>
> Let me know if this looks good & I'll check it in,
>
> - David
>
> [for the record - gcc does something rather different with this invalid
> construct. It produces special "invalid type" template arguments for
> non-final parameter packs & then any instantiation of the template fails to
> match any argument to that parameter & you get an extra diagnostic there,
> too. I'm not entirely sure how Clang's error recovery works on the failure
> path (returning Invalid from CheckTemplateArgumentList) works in this case
> or how it compares to GCC's behavior.
>
> I was considering having the original template parsing code actually modify
> the template parameter list when it hit this invalid case - moving the
> parameter pack to the end (or removing it in the case of multiple parameter
> packs) for example - but I wasn't sure how to mutate the template parameter
> list & whether that would be valid. Not to mention it could produce some
> rather strange diagnostics later on. Though it would be more indicative of a
> legitimate 'error recovery' scenario (the compiler assuming some valid
> interpretation of the invalid code & then continuing on its way - rather
> than just glossing over the details of this invalid template whenever it's
> used) & could be supported by appropriate FixItHints... (might be nice to
> add the FixIt anyway at some point (to move the parameter pack to the end))]
>
Index: test/CXX/temp/temp.param/p11-0x.cpp
===================================================================
--- test/CXX/temp/temp.param/p11-0x.cpp (revision 141939)
+++ test/CXX/temp/temp.param/p11-0x.cpp (working copy)
@@ -24,8 +24,9 @@
// If a template-parameter of a primary class template or alias template is a
// template parameter pack, it shall be the last template-parameter.
template<typename ...Types, // expected-error{{template parameter pack must be the last template parameter}}
- int After>
+ int After, int After2>
struct X0t;
+X0t<int> pr9789();
template<typename ...Types, // expected-error{{template parameter pack must be the last template parameter}}
int After>
using A0t = int;
Index: lib/Sema/SemaTemplate.cpp
===================================================================
--- lib/Sema/SemaTemplate.cpp (revision 141939)
+++ lib/Sema/SemaTemplate.cpp (working copy)
@@ -1186,9 +1186,6 @@
bool SawDefaultArgument = false;
SourceLocation PreviousDefaultArgLoc;
- bool SawParameterPack = false;
- SourceLocation ParameterPackLoc;
-
// Dummy initialization to avoid warnings.
TemplateParameterList::iterator OldParam = NewParams->end();
if (OldParams)
@@ -1203,18 +1200,11 @@
SourceLocation OldDefaultLoc;
SourceLocation NewDefaultLoc;
- // Variables used to diagnose missing default arguments
+ // Variable used to diagnose missing default arguments
bool MissingDefaultArg = false;
- // C++0x [temp.param]p11:
- // If a template parameter of a primary class template or alias template
- // is a template parameter pack, it shall be the last template parameter.
- if (SawParameterPack &&
- (TPC == TPC_ClassTemplate || TPC == TPC_TypeAliasTemplate)) {
- Diag(ParameterPackLoc,
- diag::err_template_param_pack_must_be_last_template_parameter);
- Invalid = true;
- }
+ // Variable used to diagnose non-final parameter packs
+ bool SawParameterPack = false;
if (TemplateTypeParmDecl *NewTypeParm
= dyn_cast<TemplateTypeParmDecl>(*NewParam)) {
@@ -1234,7 +1224,6 @@
assert(!NewTypeParm->hasDefaultArgument() &&
"Parameter packs can't have a default argument!");
SawParameterPack = true;
- ParameterPackLoc = NewTypeParm->getLocation();
} else if (OldTypeParm && OldTypeParm->hasDefaultArgument() &&
NewTypeParm->hasDefaultArgument()) {
OldDefaultLoc = OldTypeParm->getDefaultArgumentLoc();
@@ -1279,7 +1268,6 @@
assert(!NewNonTypeParm->hasDefaultArgument() &&
"Parameter packs can't have a default argument!");
SawParameterPack = true;
- ParameterPackLoc = NewNonTypeParm->getLocation();
} else if (OldNonTypeParm && OldNonTypeParm->hasDefaultArgument() &&
NewNonTypeParm->hasDefaultArgument()) {
OldDefaultLoc = OldNonTypeParm->getDefaultArgumentLoc();
@@ -1304,7 +1292,6 @@
} else if (SawDefaultArgument)
MissingDefaultArg = true;
} else {
- // Check the presence of a default argument here.
TemplateTemplateParmDecl *NewTemplateParm
= cast<TemplateTemplateParmDecl>(*NewParam);
@@ -1314,6 +1301,7 @@
continue;
}
+ // Check the presence of a default argument here.
if (NewTemplateParm->hasDefaultArgument() &&
DiagnoseDefaultTemplateArgument(*this, TPC,
NewTemplateParm->getLocation(),
@@ -1327,7 +1315,6 @@
assert(!NewTemplateParm->hasDefaultArgument() &&
"Parameter packs can't have a default argument!");
SawParameterPack = true;
- ParameterPackLoc = NewTemplateParm->getLocation();
} else if (OldTemplateParm && OldTemplateParm->hasDefaultArgument() &&
NewTemplateParm->hasDefaultArgument()) {
OldDefaultLoc = OldTemplateParm->getDefaultArgument().getLocation();
@@ -1354,6 +1341,16 @@
MissingDefaultArg = true;
}
+ // C++0x [temp.param]p11:
+ // If a template parameter of a primary class template or alias template
+ // is a template parameter pack, it shall be the last template parameter.
+ if (SawParameterPack && (NewParam + 1) != NewParamEnd &&
+ (TPC == TPC_ClassTemplate || TPC == TPC_TypeAliasTemplate)) {
+ Diag((*NewParam)->getLocation(),
+ diag::err_template_param_pack_must_be_last_template_parameter);
+ Invalid = true;
+ }
+
if (RedundantDefaultArg) {
// C++ [temp.param]p12:
// A template-parameter shall not be given default arguments
@@ -3036,6 +3033,8 @@
// in arguments for non-template parameter packs.
if ((*Param)->isTemplateParameterPack()) {
+ if (!HasParameterPack)
+ return true;
if (ArgumentPack.empty())
Converted.push_back(TemplateArgument(0, 0));
else {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
