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

diff --git a/clang/lib/AST/ASTDiagnostic.cpp b/clang/lib/AST/ASTDiagnostic.cpp
index a8ab9a2..f55e032 100644 (file)
--- 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<IntegerLiteral>(E)) return false;
 
     if (UnaryOperator *UO = dyn_cast<UnaryOperator>(E))

diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 254e78f..7451497 100644 (file)
--- 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<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(TemplateDecl *Template,
   //   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(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<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(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<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(TemplateDecl *Template,
     } 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(TemplateDecl *Template,
         = 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(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 bdd6d62..9f4806f 100644 (file)
--- a/clang/test/Misc/diag-template-diffing.cpp
+++ b/clang/test/Misc/diag-template-diffing.cpp
@@ -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.

diff --git a/clang/test/SemaTemplate/overloaded-functions.cpp b/clang/test/SemaTemplate/overloaded-functions.cpp
new file mode 100644 (file)
index 0000000..2656575
--- /dev/null
+++ b/clang/test/SemaTemplate/overloaded-functions.cpp
@@ -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@-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 <bool, typename>
+void Bar() {}
+
+template <int size>
+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}}
+}
+