From 15b66535cae9ec01c643b5dde8c45d468ab1ee67 Mon Sep 17 00:00:00 2001 From: Richard Trieu Date: Sat, 24 Jan 2015 02:48:32 +0000 Subject: [PATCH] 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. llvm-svn: 226983 --- clang/lib/AST/ASTDiagnostic.cpp | 36 +++++------- clang/lib/Sema/SemaTemplate.cpp | 56 +++++++++++-------- clang/test/Misc/diag-template-diffing.cpp | 14 +++++ .../SemaTemplate/overloaded-functions.cpp | 32 +++++++++++ 4 files changed, 94 insertions(+), 44 deletions(-) create mode 100644 clang/test/SemaTemplate/overloaded-functions.cpp diff --git a/clang/lib/AST/ASTDiagnostic.cpp b/clang/lib/AST/ASTDiagnostic.cpp index a8ab9a2d77b0..f55e0326c44a 100644 --- a/clang/lib/AST/ASTDiagnostic.cpp +++ b/clang/lib/AST/ASTDiagnostic.cpp @@ -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(E)) return false; if (UnaryOperator *UO = dyn_cast(E)) diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 254e78fa017b..745149726aa6 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -3355,7 +3355,7 @@ Sema::SubstDefaultTemplateArgumentIfAvailable(TemplateDecl *Template, /// \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(NamedDecl *Param, 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(TemplateDecl *Template, TemplateArgumentListInfo &TemplateArgs, bool PartialTemplateArgs, SmallVectorImpl &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(TemplateDecl *Template, // template-parameter-list. bool isTemplateTemplateParameter = isa(Template); SmallVector 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(TemplateDecl *Template, 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(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(TemplateDecl *Template, } while (ArgIdx < NumArgs) { - Converted.push_back(TemplateArgs[ArgIdx].getArgument()); + Converted.push_back(NewArgs[ArgIdx].getArgument()); ++ArgIdx; } @@ -3784,8 +3796,7 @@ bool Sema::CheckTemplateArgumentList(TemplateDecl *Template, // the default argument. if (TemplateTypeParmDecl *TTP = dyn_cast(*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(TemplateDecl *Template, } else if (NonTypeTemplateParmDecl *NTTP = dyn_cast(*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(TemplateDecl *Template, = cast(*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(TemplateDecl *Template, 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(TemplateDecl *Template, // 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; } diff --git a/clang/test/Misc/diag-template-diffing.cpp b/clang/test/Misc/diag-template-diffing.cpp index bdd6d62fdda2..9f4806f64c90 100644 --- a/clang/test/Misc/diag-template-diffing.cpp +++ b/clang/test/Misc/diag-template-diffing.cpp @@ -1247,6 +1247,20 @@ template using A7 = A<(T::num)>; // CHECK-ELIDE-NOTREE: type alias template redefinition with different types ('A<(T::num), (default) 0>' vs 'A') } +namespace TemplateArgumentImplicitConversion { +template 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. diff --git a/clang/test/SemaTemplate/overloaded-functions.cpp b/clang/test/SemaTemplate/overloaded-functions.cpp new file mode 100644 index 000000000000..26565753dd73 --- /dev/null +++ b/clang/test/SemaTemplate/overloaded-functions.cpp @@ -0,0 +1,32 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +namespace { +template +void Foo() {} + +template +void Foo() { + int arr[size]; + // expected-error@-1 {{'arr' declared as an array with a negative size}} +} +} + +void test_foo() { + Foo<-1>(); + // expected-note@-1 {{in instantiation of function template specialization '(anonymous namespace)::Foo<-1>' requested here}} +} + +template +void Bar() {} + +template +void Bar() { + int arr[size]; + // expected-error@-1 {{'arr' declared as an array with a negative size}} +} + +void test_bar() { + Bar<-1>(); + // expected-note@-1 {{in instantiation of function template specialization 'Bar<-1>' requested here}} +} +