r226983 - When checking the template argument list, use a copy of that list for changes
Richard Trieu
rtrieu at google.com
Fri Jan 23 18:48:32 PST 2015
Author: rtrieu
Date: Fri Jan 23 20:48:32 2015
New Revision: 226983
URL: http://llvm.org/viewvc/llvm-project?rev=226983&view=rev
Log:
When checking the template argument list, use a copy of that list for changes
and only update the orginal list on a valid arugment list. When checking an
individual expression template argument, and conversions are required, update
the expression in the template argument. Since template arguments are
speculatively checked, the copying of the template argument list prevents
updating the template arguments when the list does not match the template.
Additionally, clean up the integer checking code in the template diffing code.
The code performs unneccessary conversions from APSInt to APInt.
Fixes PR21758.
This essentially reverts r224770 to recommits r224667 and r224668 with extra
changes to prevent the template instantiation problems seen in PR22006.
A test to catch the discovered problem is also added.
Added:
cfe/trunk/test/SemaTemplate/overloaded-functions.cpp
Modified:
cfe/trunk/lib/AST/ASTDiagnostic.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/Misc/diag-template-diffing.cpp
Modified: cfe/trunk/lib/AST/ASTDiagnostic.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDiagnostic.cpp?rev=226983&r1=226982&r2=226983&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTDiagnostic.cpp (original)
+++ cfe/trunk/lib/AST/ASTDiagnostic.cpp Fri Jan 23 20:48:32 2015
@@ -998,10 +998,6 @@ class TemplateDiff {
(!HasFromValueDecl && !HasToValueDecl)) &&
"Template argument cannot be both integer and declaration");
- unsigned ParamWidth = 128; // Safe default
- if (FromDefaultNonTypeDecl->getType()->isIntegralOrEnumerationType())
- ParamWidth = Context.getIntWidth(FromDefaultNonTypeDecl->getType());
-
if (!HasFromInt && !HasToInt && !HasFromValueDecl && !HasToValueDecl) {
Tree.SetNode(FromExpr, ToExpr);
Tree.SetDefault(FromIter.isEnd() && FromExpr, ToIter.isEnd() && ToExpr);
@@ -1013,14 +1009,14 @@ class TemplateDiff {
}
if (HasFromInt && HasToInt) {
Tree.SetNode(FromInt, ToInt, HasFromInt, HasToInt);
- Tree.SetSame(IsSameConvertedInt(ParamWidth, FromInt, ToInt));
+ Tree.SetSame(FromInt == ToInt);
Tree.SetKind(DiffTree::Integer);
} else if (HasFromInt || HasToInt) {
Tree.SetNode(FromInt, ToInt, HasFromInt, HasToInt);
Tree.SetSame(false);
Tree.SetKind(DiffTree::Integer);
} else {
- Tree.SetSame(IsEqualExpr(Context, ParamWidth, FromExpr, ToExpr) ||
+ Tree.SetSame(IsEqualExpr(Context, FromExpr, ToExpr) ||
(FromNullPtr && ToNullPtr));
Tree.SetNullPtr(FromNullPtr, ToNullPtr);
Tree.SetKind(DiffTree::Expression);
@@ -1034,8 +1030,11 @@ class TemplateDiff {
if (!HasToInt && ToExpr)
HasToInt = GetInt(Context, ToIter, ToExpr, ToInt);
Tree.SetNode(FromInt, ToInt, HasFromInt, HasToInt);
- Tree.SetSame(HasFromInt && HasToInt &&
- IsSameConvertedInt(ParamWidth, FromInt, ToInt));
+ if (HasFromInt && HasToInt) {
+ Tree.SetSame(FromInt == ToInt);
+ } else {
+ Tree.SetSame(false);
+ }
Tree.SetDefault(FromIter.isEnd() && HasFromInt,
ToIter.isEnd() && HasToInt);
Tree.SetKind(DiffTree::Integer);
@@ -1213,7 +1212,7 @@ class TemplateDiff {
/// GetInt - Retrieves the template integer argument, including evaluating
/// default arguments.
static bool GetInt(ASTContext &Context, const TSTiterator &Iter,
- Expr *ArgExpr, llvm::APInt &Int) {
+ Expr *ArgExpr, llvm::APSInt &Int) {
// Default, value-depenedent expressions require fetching
// from the desugared TemplateArgument, otherwise expression needs to
// be evaluatable.
@@ -1303,18 +1302,8 @@ class TemplateDiff {
return nullptr;
}
- /// IsSameConvertedInt - Returns true if both integers are equal when
- /// converted to an integer type with the given width.
- static bool IsSameConvertedInt(unsigned Width, const llvm::APSInt &X,
- const llvm::APSInt &Y) {
- llvm::APInt ConvertedX = X.extOrTrunc(Width);
- llvm::APInt ConvertedY = Y.extOrTrunc(Width);
- return ConvertedX == ConvertedY;
- }
-
/// IsEqualExpr - Returns true if the expressions evaluate to the same value.
- static bool IsEqualExpr(ASTContext &Context, unsigned ParamWidth,
- Expr *FromExpr, Expr *ToExpr) {
+ static bool IsEqualExpr(ASTContext &Context, Expr *FromExpr, Expr *ToExpr) {
if (FromExpr == ToExpr)
return true;
@@ -1346,7 +1335,7 @@ class TemplateDiff {
switch (FromVal.getKind()) {
case APValue::Int:
- return IsSameConvertedInt(ParamWidth, FromVal.getInt(), ToVal.getInt());
+ return FromVal.getInt() == ToVal.getInt();
case APValue::LValue: {
APValue::LValueBase FromBase = FromVal.getLValueBase();
APValue::LValueBase ToBase = ToVal.getLValueBase();
@@ -1656,11 +1645,14 @@ class TemplateDiff {
}
Unbold();
}
-
+
/// HasExtraInfo - Returns true if E is not an integer literal or the
/// negation of an integer literal
bool HasExtraInfo(Expr *E) {
if (!E) return false;
+
+ E = E->IgnoreImpCasts();
+
if (isa<IntegerLiteral>(E)) return false;
if (UnaryOperator *UO = dyn_cast<UnaryOperator>(E))
Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=226983&r1=226982&r2=226983&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Fri Jan 23 20:48:32 2015
@@ -3355,7 +3355,7 @@ Sema::SubstDefaultTemplateArgumentIfAvai
/// \param Param The template parameter against which the argument will be
/// checked.
///
-/// \param Arg The template argument.
+/// \param Arg The template argument, which may be updated due to conversions.
///
/// \param Template The template in which the template argument resides.
///
@@ -3433,6 +3433,13 @@ bool Sema::CheckTemplateArgument(NamedDe
if (Res.isInvalid())
return true;
+ // If the resulting expression is new, then use it in place of the
+ // old expression in the template argument.
+ if (Res.get() != Arg.getArgument().getAsExpr()) {
+ TemplateArgument TA(Res.get());
+ Arg = TemplateArgumentLoc(TA, Res.get());
+ }
+
Converted.push_back(Result);
break;
}
@@ -3640,9 +3647,14 @@ bool Sema::CheckTemplateArgumentList(Tem
TemplateArgumentListInfo &TemplateArgs,
bool PartialTemplateArgs,
SmallVectorImpl<TemplateArgument> &Converted) {
+ // Make a copy of the template arguments for processing. Only make the
+ // changes at the end when successful in matching the arguments to the
+ // template.
+ TemplateArgumentListInfo NewArgs = TemplateArgs;
+
TemplateParameterList *Params = Template->getTemplateParameters();
- SourceLocation RAngleLoc = TemplateArgs.getRAngleLoc();
+ SourceLocation RAngleLoc = NewArgs.getRAngleLoc();
// C++ [temp.arg]p1:
// [...] The type and form of each template-argument specified in
@@ -3651,7 +3663,7 @@ bool Sema::CheckTemplateArgumentList(Tem
// template-parameter-list.
bool isTemplateTemplateParameter = isa<TemplateTemplateParmDecl>(Template);
SmallVector<TemplateArgument, 2> ArgumentPack;
- unsigned ArgIdx = 0, NumArgs = TemplateArgs.size();
+ unsigned ArgIdx = 0, NumArgs = NewArgs.size();
LocalInstantiationScope InstScope(*this, true);
for (TemplateParameterList::iterator Param = Params->begin(),
ParamEnd = Params->end();
@@ -3687,21 +3699,21 @@ bool Sema::CheckTemplateArgumentList(Tem
if (ArgIdx < NumArgs) {
// Check the template argument we were given.
- if (CheckTemplateArgument(*Param, TemplateArgs[ArgIdx], Template,
+ if (CheckTemplateArgument(*Param, NewArgs[ArgIdx], Template,
TemplateLoc, RAngleLoc,
ArgumentPack.size(), Converted))
return true;
bool PackExpansionIntoNonPack =
- TemplateArgs[ArgIdx].getArgument().isPackExpansion() &&
+ NewArgs[ArgIdx].getArgument().isPackExpansion() &&
(!(*Param)->isTemplateParameterPack() || getExpandedPackSize(*Param));
if (PackExpansionIntoNonPack && isa<TypeAliasTemplateDecl>(Template)) {
// Core issue 1430: we have a pack expansion as an argument to an
// alias template, and it's not part of a parameter pack. This
// can't be canonicalized, so reject it now.
- Diag(TemplateArgs[ArgIdx].getLocation(),
+ Diag(NewArgs[ArgIdx].getLocation(),
diag::err_alias_template_expansion_into_fixed_list)
- << TemplateArgs[ArgIdx].getSourceRange();
+ << NewArgs[ArgIdx].getSourceRange();
Diag((*Param)->getLocation(), diag::note_template_param_here);
return true;
}
@@ -3733,7 +3745,7 @@ bool Sema::CheckTemplateArgumentList(Tem
}
while (ArgIdx < NumArgs) {
- Converted.push_back(TemplateArgs[ArgIdx].getArgument());
+ Converted.push_back(NewArgs[ArgIdx].getArgument());
++ArgIdx;
}
@@ -3784,8 +3796,7 @@ bool Sema::CheckTemplateArgumentList(Tem
// the default argument.
if (TemplateTypeParmDecl *TTP = dyn_cast<TemplateTypeParmDecl>(*Param)) {
if (!TTP->hasDefaultArgument())
- return diagnoseArityMismatch(*this, Template, TemplateLoc,
- TemplateArgs);
+ return diagnoseArityMismatch(*this, Template, TemplateLoc, NewArgs);
TypeSourceInfo *ArgType = SubstDefaultTemplateArgument(*this,
Template,
@@ -3801,8 +3812,7 @@ bool Sema::CheckTemplateArgumentList(Tem
} else if (NonTypeTemplateParmDecl *NTTP
= dyn_cast<NonTypeTemplateParmDecl>(*Param)) {
if (!NTTP->hasDefaultArgument())
- return diagnoseArityMismatch(*this, Template, TemplateLoc,
- TemplateArgs);
+ return diagnoseArityMismatch(*this, Template, TemplateLoc, NewArgs);
ExprResult E = SubstDefaultTemplateArgument(*this, Template,
TemplateLoc,
@@ -3819,8 +3829,7 @@ bool Sema::CheckTemplateArgumentList(Tem
= cast<TemplateTemplateParmDecl>(*Param);
if (!TempParm->hasDefaultArgument())
- return diagnoseArityMismatch(*this, Template, TemplateLoc,
- TemplateArgs);
+ return diagnoseArityMismatch(*this, Template, TemplateLoc, NewArgs);
NestedNameSpecifierLoc QualifierLoc;
TemplateName Name = SubstDefaultTemplateArgument(*this, Template,
@@ -3848,12 +3857,12 @@ bool Sema::CheckTemplateArgumentList(Tem
RAngleLoc, 0, Converted))
return true;
- // Core issue 150 (assumed resolution): if this is a template template
- // parameter, keep track of the default template arguments from the
+ // Core issue 150 (assumed resolution): if this is a template template
+ // parameter, keep track of the default template arguments from the
// template definition.
if (isTemplateTemplateParameter)
- TemplateArgs.addArgument(Arg);
-
+ NewArgs.addArgument(Arg);
+
// Move to the next template parameter and argument.
++Param;
++ArgIdx;
@@ -3865,15 +3874,18 @@ bool Sema::CheckTemplateArgumentList(Tem
// still dependent).
if (ArgIdx < NumArgs && CurrentInstantiationScope &&
CurrentInstantiationScope->getPartiallySubstitutedPack()) {
- while (ArgIdx < NumArgs &&
- TemplateArgs[ArgIdx].getArgument().isPackExpansion())
- Converted.push_back(TemplateArgs[ArgIdx++].getArgument());
+ while (ArgIdx < NumArgs && NewArgs[ArgIdx].getArgument().isPackExpansion())
+ Converted.push_back(NewArgs[ArgIdx++].getArgument());
}
// If we have any leftover arguments, then there were too many arguments.
// Complain and fail.
if (ArgIdx < NumArgs)
- return diagnoseArityMismatch(*this, Template, TemplateLoc, TemplateArgs);
+ return diagnoseArityMismatch(*this, Template, TemplateLoc, NewArgs);
+
+ // No problems found with the new argument list, propagate changes back
+ // to caller.
+ TemplateArgs = NewArgs;
return false;
}
Modified: cfe/trunk/test/Misc/diag-template-diffing.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/diag-template-diffing.cpp?rev=226983&r1=226982&r2=226983&view=diff
==============================================================================
--- cfe/trunk/test/Misc/diag-template-diffing.cpp (original)
+++ cfe/trunk/test/Misc/diag-template-diffing.cpp Fri Jan 23 20:48:32 2015
@@ -1247,6 +1247,20 @@ template <class T> using A7 = A<(T::num)
// CHECK-ELIDE-NOTREE: type alias template redefinition with different types ('A<(T::num), (default) 0>' vs 'A<T::num, 1>')
}
+namespace TemplateArgumentImplicitConversion {
+template <int X> struct condition {};
+
+struct is_const {
+ constexpr operator int() const { return 10; }
+};
+
+using T = condition<(is_const())>;
+void foo(const T &t) {
+ T &t2 = t;
+}
+// CHECK-ELIDE-NOTREE: binding of reference to type 'condition<[...]>' to a value of type 'const condition<[...]>' drops qualifiers
+}
+
// CHECK-ELIDE-NOTREE: {{[0-9]*}} errors generated.
// CHECK-NOELIDE-NOTREE: {{[0-9]*}} errors generated.
// CHECK-ELIDE-TREE: {{[0-9]*}} errors generated.
Added: cfe/trunk/test/SemaTemplate/overloaded-functions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/overloaded-functions.cpp?rev=226983&view=auto
==============================================================================
--- cfe/trunk/test/SemaTemplate/overloaded-functions.cpp (added)
+++ cfe/trunk/test/SemaTemplate/overloaded-functions.cpp Fri Jan 23 20:48:32 2015
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+namespace {
+template <bool, typename>
+void Foo() {}
+
+template <int size>
+void Foo() {
+ int arr[size];
+ // expected-error at -1 {{'arr' declared as an array with a negative size}}
+}
+}
+
+void test_foo() {
+ Foo<-1>();
+ // expected-note at -1 {{in instantiation of function template specialization '(anonymous namespace)::Foo<-1>' requested here}}
+}
+
+template <bool, typename>
+void Bar() {}
+
+template <int size>
+void Bar() {
+ int arr[size];
+ // expected-error at -1 {{'arr' declared as an array with a negative size}}
+}
+
+void test_bar() {
+ Bar<-1>();
+ // expected-note at -1 {{in instantiation of function template specialization 'Bar<-1>' requested here}}
+}
+
More information about the cfe-commits
mailing list