[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: [PATCHES] crash fixes in TemplateDiff (diagnostics)
From: Olivier Goffart <ogoffart () kde ! org>
Date: 2013-04-02 21:46:29
Message-ID: 3536410.5Dk7lgf3NE () gargamel
[Download RAW message or body]
Hi,
Here is a set of patches that fixes a couple of crash when producing
diagnostics with some template constructs.
(For example http://llvm.org/bugs/show_bug.cgi?id=15491 )
Regards
--
Olivier
["0001-Fix-assert-in-DiffTemplate-when-default-template-arg.patch" (0001-Fix-assert-in-DiffTemplate-when-default-template-arg.patch)]
From f51c329dc66361209381799f4829a3366254631b Mon Sep 17 00:00:00 2001
From: Olivier Goffart <ogoffart@woboq.com>
Date: Tue, 2 Apr 2013 18:49:48 +0200
Subject: [PATCH 1/3] Fix assert in DiffTemplate when default template
argument is dependent
In code like this:
template <class> struct Trait { enum { V }; };
template <typename T, int = Trait<T>::V> struct K { };
int main() {
K<int> f;
K<char> f2;
f2 = f;
}
We could end up calling FromExpr->EvaluateKnownConstInt while FromExpr
is a type debendent expression. (Which would assert or crash)
We need to use the desugar expression in all cases.
The code that take the desugar type was moved into a function since it
is used 4 times.
The same needs to be done for Declaration expressions
---
lib/AST/ASTDiagnostic.cpp | 99 ++++++++++++++++++-------------------
test/Misc/diag-template-diffing.cpp | 55 +++++++++++++++++++++
2 files changed, 103 insertions(+), 51 deletions(-)
diff --git a/lib/AST/ASTDiagnostic.cpp b/lib/AST/ASTDiagnostic.cpp
index b9c57ab..d966f04 100644
--- a/lib/AST/ASTDiagnostic.cpp
+++ b/lib/AST/ASTDiagnostic.cpp
@@ -913,47 +913,11 @@ class TemplateDiff {
ToIter.isEnd() && ToExpr);
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);
+ if (FromExpr)
+ FromInt = GetIntFromExpr(FromIter, FromExpr);
+ if (ToExpr)
+ ToInt = GetIntFromExpr(ToIter, ToExpr);
+ Tree.SetNode(FromInt, ToInt, FromExpr, ToExpr);
Tree.SetSame(IsSameConvertedInt(ParamWidth, FromInt, ToInt));
Tree.SetKind(DiffTree::Integer);
} else {
@@ -962,11 +926,11 @@ class TemplateDiff {
}
} else if (HasFromInt || HasToInt) {
if (!HasFromInt && FromExpr) {
- FromInt = FromExpr->EvaluateKnownConstInt(Context);
+ FromInt = GetIntFromExpr(FromIter, FromExpr);
HasFromInt = true;
}
if (!HasToInt && ToExpr) {
- ToInt = ToExpr->EvaluateKnownConstInt(Context);
+ ToInt = GetIntFromExpr(ToIter, ToExpr);
HasToInt = true;
}
Tree.SetNode(FromInt, ToInt, HasFromInt, HasToInt);
@@ -975,14 +939,11 @@ class TemplateDiff {
ToIter.isEnd() && HasToInt);
Tree.SetKind(DiffTree::Integer);
} else {
- if (!HasFromValueDecl && FromExpr) {
- DeclRefExpr *DRE = cast<DeclRefExpr>(FromExpr);
- FromValueDecl = cast<ValueDecl>(DRE->getDecl());
- }
- if (!HasToValueDecl && ToExpr) {
- DeclRefExpr *DRE = cast<DeclRefExpr>(ToExpr);
- ToValueDecl = cast<ValueDecl>(DRE->getDecl());
- }
+ if (!HasFromValueDecl && FromExpr)
+ FromValueDecl = GetValueDeclFromExpr(FromIter, FromExpr);
+ if (!HasToValueDecl && ToExpr)
+ ToValueDecl = GetValueDeclFromExpr(ToIter, ToExpr);
+
Tree.SetNode(FromValueDecl, ToValueDecl);
Tree.SetSame(FromValueDecl->getCanonicalDecl() ==
ToValueDecl->getCanonicalDecl());
@@ -1101,6 +1062,42 @@ class TemplateDiff {
ArgExpr = SNTTPE->getReplacement();
}
+ /// GetInt - Retrieves the template integer argument from a default
+ /// expression argument argument.
+ llvm::APInt GetIntFromExpr(const TSTiterator &Iter, Expr *ArgExpr) {
+ // Default, value-depenedent expressions require fetching
+ // from the desugared TemplateArgument
+ if (Iter.isEnd() && ArgExpr->isValueDependent())
+ switch (Iter.getDesugar().getKind()) {
+ case TemplateArgument::Integral:
+ return Iter.getDesugar().getAsIntegral();
+ case TemplateArgument::Expression:
+ ArgExpr = Iter.getDesugar().getAsExpr();
+ return ArgExpr->EvaluateKnownConstInt(Context);
+ default:
+ assert(0 && "Unexpected template argument kind");
+ }
+ return ArgExpr->EvaluateKnownConstInt(Context);
+ }
+
+ /// GetValueDecl - Retrieves the template integer argument from a default
+ /// expression argument argument.
+ ValueDecl *GetValueDeclFromExpr(const TSTiterator &Iter, Expr *ArgExpr) {
+ // Default, value-depenedent expressions require fetching
+ // from the desugared TemplateArgument
+ if (Iter.isEnd() && ArgExpr->isValueDependent())
+ switch (Iter.getDesugar().getKind()) {
+ case TemplateArgument::Declaration:
+ return Iter.getDesugar().getAsDecl();
+ case TemplateArgument::Expression:
+ ArgExpr = Iter.getDesugar().getAsExpr();
+ return cast<DeclRefExpr>(ArgExpr)->getDecl();
+ default:
+ assert(0 && "Unexpected template argument kind");
+ }
+ return cast<DeclRefExpr>(ArgExpr)->getDecl();
+ }
+
/// GetTemplateDecl - Retrieves the template template arguments, including
/// default arguments.
void GetTemplateDecl(const TSTiterator &Iter,
diff --git a/test/Misc/diag-template-diffing.cpp \
b/test/Misc/diag-template-diffing.cpp index 823bad2..fd33113 100644
--- a/test/Misc/diag-template-diffing.cpp
+++ b/test/Misc/diag-template-diffing.cpp
@@ -903,6 +903,61 @@ namespace ValueDecl {
}
}
+namespace DependentDefault {
+ template <typename> struct Trait {
+ enum { V = 40 };
+ typedef int Ty;
+ static int I;
+ };
+ int other;
+
+ template <typename T, int = Trait<T>::V > struct A {};
+ template <typename T, typename = Trait<T>::Ty > struct B {};
+ template <typename T, int& = Trait<T>::I > struct C {};
+
+ void test() {
+
+ A<int> a1;
+ A<char> a2;
+ A<int, 10> a3;
+ a1 = a2;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'A<char, [...]>' to 'A<int, \
[...]>' + a3 = a1;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'A<[...], (default) 40>' to \
'A<[...], 10>' + a2 = a3;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'A<int, 10>' to 'A<char, 40>'
+
+ B<int> b1;
+ B<char> b2;
+ B<int, char> b3;
+ b1 = b2;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'B<char, (default) \
Trait<T>::Ty>' to 'B<int, int>' + b3 = b1;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'B<[...], (default) \
Trait<T>::Ty>' to 'B<[...], char>' + b2 = b3;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'B<int, char>' to 'B<char, int>'
+
+ C<int> c1;
+ C<char> c2;
+ C<int, other> c3;
+ c1 = c2;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'C<char, (default) I>' to \
'C<int, I>' + c3 = c1;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'C<[...], (default) I>' to \
'C<[...], other>' + c2 = c3;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'C<int, other>' to 'C<char, I>'
+ }
+}
+
// CHECK-ELIDE-NOTREE: {{[0-9]*}} errors generated.
// CHECK-NOELIDE-NOTREE: {{[0-9]*}} errors generated.
// CHECK-ELIDE-TREE: {{[0-9]*}} errors generated.
--
1.7.12.1
["0002-Fix-crash-in-TemplateDiff-when-comparing-variadic-te.patch" (0002-Fix-crash-in-TemplateDiff-when-comparing-variadic-te.patch)]
From 1f4e52c914f9cc154de90d2c18198c98f00ffed0 Mon Sep 17 00:00:00 2001
From: Olivier Goffart <ogoffart@woboq.com>
Date: Tue, 2 Apr 2013 19:29:34 +0200
Subject: [PATCH 2/3] Fix crash in TemplateDiff when comparing variadic
template of declarations
If the From and the To type have different lenght, the ValueDecl may be
null.
---
lib/AST/ASTDiagnostic.cpp | 5 +++--
test/Misc/diag-template-diffing.cpp | 39 +++++++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/lib/AST/ASTDiagnostic.cpp b/lib/AST/ASTDiagnostic.cpp
index d966f04..7a95901 100644
--- a/lib/AST/ASTDiagnostic.cpp
+++ b/lib/AST/ASTDiagnostic.cpp
@@ -945,8 +945,9 @@ class TemplateDiff {
ToValueDecl = GetValueDeclFromExpr(ToIter, ToExpr);
Tree.SetNode(FromValueDecl, ToValueDecl);
- Tree.SetSame(FromValueDecl->getCanonicalDecl() ==
- ToValueDecl->getCanonicalDecl());
+ Tree.SetSame(FromValueDecl && ToValueDecl &&
+ FromValueDecl->getCanonicalDecl() ==
+ ToValueDecl->getCanonicalDecl());
Tree.SetDefault(FromIter.isEnd() && FromValueDecl,
ToIter.isEnd() && ToValueDecl);
Tree.SetKind(DiffTree::Declaration);
diff --git a/test/Misc/diag-template-diffing.cpp \
b/test/Misc/diag-template-diffing.cpp index fd33113..f27f8b6 100644
--- a/test/Misc/diag-template-diffing.cpp
+++ b/test/Misc/diag-template-diffing.cpp
@@ -958,6 +958,45 @@ namespace DependentDefault {
}
}
+namespace VariadicDefault {
+ int i1, i2, i3;
+ template <int = 5, int...> struct A {};
+ template <int& = i1, int& ...> struct B {};
+ template <typename = void, typename...> struct C {};
+
+ void test() {
+ A<> a1;
+ A<5, 6, 7> a2;
+ A<1, 2> a3;
+ a2 = a1;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'A<[...], (no argument), (no \
argument)>' to 'A<[...], 6, 7>' + a3 = a1;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'A<(default) 5, (no argument)>' \
to 'A<1, 2>' +
+ B<> b1;
+ B<i1, i2, i3> b2;
+ B<i2, i3> b3;
+ b2 = b1;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'B<[...], (no argument), (no \
argument)>' to 'B<[...], i2, i3>' + b3 = b1;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'B<(default) i1, (no argument)>' \
to 'B<i2, i3>' +
+ C<> c1;
+ C<void, void> c2;
+ C<char, char> c3;
+ c2 = c1;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'C<[...], (no argument)>' to \
'C<[...], void>' + c3 = c1;
+ // CHECK-ELIDE-NOTREE: no viable overloaded '='
+ // CHECK-ELIDE-NOTREE: no known conversion from 'C<(default) void, (no \
argument)>' to 'C<char, char>' + }
+}
+
// CHECK-ELIDE-NOTREE: {{[0-9]*}} errors generated.
// CHECK-NOELIDE-NOTREE: {{[0-9]*}} errors generated.
// CHECK-ELIDE-TREE: {{[0-9]*}} errors generated.
--
1.7.12.1
["0003-Fix-crash-in-DiffTemplate.patch" (0003-Fix-crash-in-DiffTemplate.patch)]
From 194a0a92571a42e72e4f82743a1deab99d50be87 Mon Sep 17 00:00:00 2001
From: Olivier Goffart <ogoffart@woboq.com>
Date: Tue, 2 Apr 2013 23:21:46 +0200
Subject: [PATCH 3/3] Fix crash in DiffTemplate
Do not assume the template argument is an integer only because the
expressions are integer. It can also be ValueDecl expressions
Use the type information from the TemplateParameterList instead
---
lib/AST/ASTDiagnostic.cpp | 3 +--
test/Misc/diag-template-diffing.cpp | 5 +++++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/lib/AST/ASTDiagnostic.cpp b/lib/AST/ASTDiagnostic.cpp
index 7a95901..65e26ce 100644
--- a/lib/AST/ASTDiagnostic.cpp
+++ b/lib/AST/ASTDiagnostic.cpp
@@ -911,8 +911,7 @@ class TemplateDiff {
Tree.SetNode(FromExpr, ToExpr);
Tree.SetDefault(FromIter.isEnd() && FromExpr,
ToIter.isEnd() && ToExpr);
- if ((FromExpr && FromExpr->getType()->isIntegerType()) ||
- (ToExpr && ToExpr->getType()->isIntegerType())) {
+ if (DefaultNTTPD->getType()->isIntegralOrEnumerationType()) {
if (FromExpr)
FromInt = GetIntFromExpr(FromIter, FromExpr);
if (ToExpr)
diff --git a/test/Misc/diag-template-diffing.cpp \
b/test/Misc/diag-template-diffing.cpp index f27f8b6..aa9483c 100644
--- a/test/Misc/diag-template-diffing.cpp
+++ b/test/Misc/diag-template-diffing.cpp
@@ -984,6 +984,11 @@ namespace VariadicDefault {
b3 = b1;
// CHECK-ELIDE-NOTREE: no viable overloaded '='
// CHECK-ELIDE-NOTREE: no known conversion from 'B<(default) i1, (no argument)>' \
to 'B<i2, i3>' + B<i1, i2, i3> b4 = b1;
+ // CHECK-ELIDE-NOTREE: no viable conversion from 'B<[...], (no argument), (no \
argument)>' to 'B<[...], i2, i3>' + B<i2, i3> b5 = b1;
+ // CHECK-ELIDE-NOTREE: no viable conversion from 'B<(default) i1, (no \
argument)>' to 'B<i2, i3>' +
C<> c1;
C<void, void> c2;
--
1.7.12.1
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic