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
diff --git a/lib/AST/ASTDiagnostic.cpp b/lib/AST/ASTDiagnostic.cpp
index d4c96cf..b1d67f8 100644
--- a/lib/AST/ASTDiagnostic.cpp
+++ b/lib/AST/ASTDiagnostic.cpp
@@ -689,6 +689,10 @@
/// 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 @@
/// 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 @@
/// 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 @@
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 @@
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 @@
// 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 @@
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 @@
Tree.SetKind(DiffTree::TemplateTemplate);
}
- if (!FromIter.isEnd()) ++FromIter;
- if (!ToIter.isEnd()) ++ToIter;
+ ++FromIter;
+ ++ToIter;
Tree.Up();
}
}
@@ -1157,10 +1220,13 @@
}
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 @@
/// 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 @@
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 2c22841..c771857 100644
--- a/test/Misc/diag-template-diffing-color.cpp
+++ b/test/Misc/diag-template-diffing-color.cpp
@@ -70,3 +70,17 @@
// 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 f374fbc..9d0439c 100644
--- a/test/Misc/diag-template-diffing-cxx98.cpp
+++ b/test/Misc/diag-template-diffing-cxx98.cpp
@@ -11,7 +11,23 @@
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 6be7055..ac15dfe 100644
--- a/test/Misc/diag-template-diffing.cpp
+++ b/test/Misc/diag-template-diffing.cpp
@@ -797,7 +797,7 @@
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 {
