Improve template diffing handling of default integer values.

When the template argument is both default and value dependent, the expression
retrieved for the default argument cannot be evaluated, thus never matching
any argument value.  To get the proper value, get the template argument
from the desugared template specialization.  Also, output the original
expression to provide more information about the argument mismatch.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177209 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 145 insertions, 24 deletions

diff --git a/lib/AST/ASTDiagnostic.cpp b/lib/AST/ASTDiagnostic.cpp
index d4c96cf01e..b1d67f8a2b 100644
--- a/lib/AST/ASTDiagnostic.cpp
+++ b/lib/AST/ASTDiagnostic.cpp
@@ -689,6 +689,10 @@ class TemplateDiff {
     /// traverse over.
     const TemplateSpecializationType *TST;
 
+    /// DesugarTST - desugared template specialization used to extract
+    /// default argument information
+    const TemplateSpecializationType *DesugarTST;
+
     /// Index - the index of the template argument in TST.
     unsigned Index;
 
@@ -701,8 +705,10 @@ class TemplateDiff {
 
     /// TSTiterator - Constructs an iterator and sets it to the first template
     /// argument.
-    TSTiterator(const TemplateSpecializationType *TST)
-        : TST(TST), Index(0), CurrentTA(0), EndTA(0) {
+    TSTiterator(ASTContext &Context, const TemplateSpecializationType *TST)
+        : TST(TST),
+          DesugarTST(GetTemplateSpecializationType(Context, TST->desugar())),
+          Index(0), CurrentTA(0), EndTA(0) {
       if (isEnd()) return;
 
       // Set to first template argument.  If not a parameter pack, done.
@@ -723,12 +729,17 @@ class TemplateDiff {
 
     /// isEnd - Returns true if the iterator is one past the end.
     bool isEnd() const {
-      return Index == TST->getNumArgs();
+      return Index >= TST->getNumArgs();
     }
 
     /// &operator++ - Increment the iterator to the next template argument.
     TSTiterator &operator++() {
-      assert(!isEnd() && "Iterator incremented past end of arguments.");
+      // After the end, Index should be the default argument position in
+      // DesugarTST, if it exists.
+      if (isEnd()) {
+        ++Index;
+        return *this;
+      }
 
       // If in a parameter pack, advance in the parameter pack.
       if (CurrentTA != EndTA) {
@@ -769,6 +780,11 @@ class TemplateDiff {
     pointer operator->() const {
       return &operator*();
     }
+
+    /// getDesugar - Returns the deduced template argument from DesguarTST
+    reference getDesugar() const {
+      return DesugarTST->getArg(Index);
+    }
   };
 
   // These functions build up the template diff tree, including functions to
@@ -808,7 +824,7 @@ class TemplateDiff {
     TemplateParameterList *Params =
         FromTST->getTemplateName().getAsTemplateDecl()->getTemplateParameters();
     unsigned TotalArgs = 0;
-    for (TSTiterator FromIter(FromTST), ToIter(ToTST);
+    for (TSTiterator FromIter(Context, FromTST), ToIter(Context, ToTST);
          !FromIter.isEnd() || !ToIter.isEnd(); ++TotalArgs) {
       Tree.AddNode();
 
@@ -856,7 +872,7 @@ class TemplateDiff {
       // Handle Expressions
       if (NonTypeTemplateParmDecl *DefaultNTTPD =
               dyn_cast<NonTypeTemplateParmDecl>(ParamND)) {
-        Expr *FromExpr, *ToExpr;
+        Expr *FromExpr = 0, *ToExpr = 0;
         llvm::APSInt FromInt, ToInt;
         ValueDecl *FromValueDecl = 0, *ToValueDecl = 0;
         unsigned ParamWidth = 128; // Safe default
@@ -893,10 +909,57 @@ class TemplateDiff {
 
         if (!HasFromInt && !HasToInt && !HasFromValueDecl && !HasToValueDecl) {
           Tree.SetNode(FromExpr, ToExpr);
-          Tree.SetSame(IsEqualExpr(Context, ParamWidth, FromExpr, ToExpr));
           Tree.SetDefault(FromIter.isEnd() && FromExpr,
                           ToIter.isEnd() && ToExpr);
-          Tree.SetKind(DiffTree::Expression);
+          if ((FromExpr && FromExpr->getType()->isIntegerType()) ||
+              (ToExpr && ToExpr->getType()->isIntegerType())) {
+            HasFromInt = FromExpr;
+            HasToInt = ToExpr;
+            if (FromExpr) {
+              // Getting the integer value from the expression.
+              // Default, value-depenedent expressions require fetching
+              // from the desugared TemplateArgument
+              if (FromIter.isEnd() && FromExpr->isValueDependent())
+                switch (FromIter.getDesugar().getKind()) {
+                  case TemplateArgument::Integral:
+                    FromInt = FromIter.getDesugar().getAsIntegral();
+                    break;
+                  case TemplateArgument::Expression:
+                    FromExpr = FromIter.getDesugar().getAsExpr();
+                    FromInt = FromExpr->EvaluateKnownConstInt(Context);
+                    break;
+                  default:
+                    assert(0 && "Unexpected template argument kind");
+                }
+              else
+                FromInt = FromExpr->EvaluateKnownConstInt(Context);
+            }
+            if (ToExpr) {
+              // Getting the integer value from the expression.
+              // Default, value-depenedent expressions require fetching
+              // from the desugared TemplateArgument
+              if (ToIter.isEnd() && ToExpr->isValueDependent())
+                switch (ToIter.getDesugar().getKind()) {
+                  case TemplateArgument::Integral:
+                    ToInt = ToIter.getDesugar().getAsIntegral();
+                    break;
+                  case TemplateArgument::Expression:
+                    ToExpr = ToIter.getDesugar().getAsExpr();
+                    ToInt = ToExpr->EvaluateKnownConstInt(Context);
+                    break;
+                  default:
+                    assert(0 && "Unexpected template argument kind");
+                }
+              else
+                ToInt = ToExpr->EvaluateKnownConstInt(Context);
+            }
+            Tree.SetNode(FromInt, ToInt, HasFromInt, HasToInt);
+            Tree.SetSame(IsSameConvertedInt(ParamWidth, FromInt, ToInt));
+            Tree.SetKind(DiffTree::Integer);
+          } else {
+            Tree.SetSame(IsEqualExpr(Context, ParamWidth, FromExpr, ToExpr));
+            Tree.SetKind(DiffTree::Expression);
+          }
         } else if (HasFromInt || HasToInt) {
           if (!HasFromInt && FromExpr) {
             FromInt = FromExpr->EvaluateKnownConstInt(Context);
@@ -942,8 +1005,8 @@ class TemplateDiff {
         Tree.SetKind(DiffTree::TemplateTemplate);
       }
 
-      if (!FromIter.isEnd()) ++FromIter;
-      if (!ToIter.isEnd()) ++ToIter;
+      ++FromIter;
+      ++ToIter;
       Tree.Up();
     }
   }
@@ -1157,10 +1220,13 @@ class TemplateDiff {
       }
       case DiffTree::Integer: {
         llvm::APSInt FromInt, ToInt;
+        Expr *FromExpr, *ToExpr;
         bool IsValidFromInt, IsValidToInt;
+        Tree.GetNode(FromExpr, ToExpr);
         Tree.GetNode(FromInt, ToInt, IsValidFromInt, IsValidToInt);
         PrintAPSInt(FromInt, ToInt, IsValidFromInt, IsValidToInt,
-                    Tree.FromDefault(), Tree.ToDefault(), Tree.NodeIsSame());
+                    FromExpr, ToExpr, Tree.FromDefault(), Tree.ToDefault(),
+                    Tree.NodeIsSame());
         return;
       }
       case DiffTree::Declaration: {
@@ -1361,8 +1427,8 @@ class TemplateDiff {
   /// PrintAPSInt - Handles printing of integral arguments, highlighting
   /// argument differences.
   void PrintAPSInt(llvm::APSInt FromInt, llvm::APSInt ToInt,
-                   bool IsValidFromInt, bool IsValidToInt, bool FromDefault,
-                   bool ToDefault, bool Same) {
+                   bool IsValidFromInt, bool IsValidToInt, Expr *FromExpr,
+                   Expr *ToExpr, bool FromDefault, bool ToDefault, bool Same) {
     assert((IsValidFromInt || IsValidToInt) &&
            "Only one integral argument may be missing.");
 
@@ -1370,23 +1436,48 @@ class TemplateDiff {
       OS << FromInt.toString(10);
     } else if (!PrintTree) {
       OS << (FromDefault ? "(default) " : "");
-      Bold();
-      OS << (IsValidFromInt ? FromInt.toString(10) : "(no argument)");
-      Unbold();
+      PrintAPSInt(FromInt, FromExpr, IsValidFromInt);
     } else {
       OS << (FromDefault ? "[(default) " : "[");
-      Bold();
-      OS << (IsValidFromInt ? FromInt.toString(10) : "(no argument)");
-      Unbold();
+      PrintAPSInt(FromInt, FromExpr, IsValidFromInt);
       OS << " != " << (ToDefault ? "(default) " : "");
-      Bold();
-      OS << (IsValidToInt ? ToInt.toString(10) : "(no argument)");
-      Unbold();
+      PrintAPSInt(ToInt, ToExpr, IsValidToInt);
       OS << ']';
     }
   }
 
+  /// PrintAPSInt - If valid, print the APSInt.  If the expression is
+  /// gives more information, print it too.
+  void PrintAPSInt(llvm::APSInt Val, Expr *E, bool Valid) {
+    Bold();
+    if (Valid) {
+      if (HasExtraInfo(E)) {
+        PrintExpr(E);
+        Unbold();
+        OS << " aka ";
+        Bold();
+      }
+      OS << Val.toString(10);
+    } else {
+      OS << "(no argument)";
+    }
+    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;
+    if (isa<IntegerLiteral>(E)) return false;
+
+    if (UnaryOperator *UO = dyn_cast<UnaryOperator>(E))
+      if (UO->getOpcode() == UO_Minus)
+        if (isa<IntegerLiteral>(UO->getSubExpr()))
+          return false;
+
+    return true;
+  }
+
   /// PrintDecl - Handles printing of Decl arguments, highlighting
   /// argument differences.
   void PrintValueDecl(ValueDecl *FromValueDecl, ValueDecl *ToValueDecl,
diff --git a/test/Misc/diag-template-diffing-color.cpp b/test/Misc/diag-template-diffing-color.cpp
index 2c228414b3..c771857f39 100644
--- a/test/Misc/diag-template-diffing-color.cpp
+++ b/test/Misc/diag-template-diffing-color.cpp
@@ -70,3 +70,17 @@ void test19() {
 // TREE:   vector<
 // TREE:     [const != const [[CYAN]]volatile[[RESET]]] vector<
 // TREE:       [...]>>
+
+namespace default_args {
+  template <int x, int y = 1+1, int z = 2>
+  class A {};
+
+  void foo(A<0> &M) {
+    // CHECK: no viable conversion from 'A<[...], (default) [[CYAN]]1 + 1[[RESET]][[BOLD]] aka [[CYAN]]2[[RESET]][[BOLD]], (default) [[CYAN]]2[[RESET]][[BOLD]]>' to 'A<[...], [[CYAN]]0[[RESET]][[BOLD]], [[CYAN]]0[[RESET]][[BOLD]]>'
+    A<0, 0, 0> N = M;
+
+    // CHECK: no viable conversion from 'A<[2 * ...], (default) [[CYAN]]2[[RESET]][[BOLD]]>' to 'A<[2 * ...], [[CYAN]]0[[RESET]][[BOLD]]>'
+    A<0, 2, 0> N2 = M;
+  }
+
+}
diff --git a/test/Misc/diag-template-diffing-cxx98.cpp b/test/Misc/diag-template-diffing-cxx98.cpp
index f374fbc417..9d0439c282 100644
--- a/test/Misc/diag-template-diffing-cxx98.cpp
+++ b/test/Misc/diag-template-diffing-cxx98.cpp
@@ -11,7 +11,23 @@ namespace PR15513 {
   class A {};
 
   void foo(A<0> &M) {
-    // CHECK: no viable conversion from 'A<[...], (default) x + 1>' to 'A<[...], 0>'
+    // CHECK: no viable conversion from 'A<[...], (default) x + 1 aka 1>' to 'A<[...], 0>'
     A<0, 0> N = M;
+   // CHECK: no viable conversion from 'A<0, [...]>' to 'A<1, [...]>'
+    A<1, 1> O = M;
   }
 }
+
+namespace default_args {
+  template <int x, int y = 1+1, int z = 2>
+  class A {};
+
+  void foo(A<0> &M) {
+    // CHECK: no viable conversion from 'A<[...], (default) 1 + 1 aka 2, (default) 2>' to 'A<[...], 0, 0>'
+    A<0, 0, 0> N = M;
+
+    // CHECK: no viable conversion from 'A<[2 * ...], (default) 2>' to 'A<[2 * ...], 0>'
+    A<0, 2, 0> N2 = M;
+  }
+
+}
diff --git a/test/Misc/diag-template-diffing.cpp b/test/Misc/diag-template-diffing.cpp
index 6be705511d..ac15dfe29c 100644
--- a/test/Misc/diag-template-diffing.cpp
+++ b/test/Misc/diag-template-diffing.cpp
@@ -797,7 +797,7 @@ namespace PR14342 {
   X<int, (signed char)-1> x = X<long, -1>();
   X<int, 3UL> y = X<int, 2>();
   // CHECK-ELIDE-NOTREE: error: no viable conversion from 'X<long, [...]>' to 'X<int, [...]>'
-  // CHECK-ELIDE-NOTREE: error: no viable conversion from 'X<[...], 2>' to 'X<[...], 3UL>'
+  // CHECK-ELIDE-NOTREE: error: no viable conversion from 'X<[...], 2>' to 'X<[...], 3>'
 }
 
 namespace PR14489 {