From cfe-commits Tue Apr 02 21:46:29 2013 From: Olivier Goffart Date: Tue, 02 Apr 2013 21:46:29 +0000 To: cfe-commits Subject: [PATCHES] crash fixes in TemplateDiff (diagnostics) Message-Id: <3536410.5Dk7lgf3NE () gargamel> X-MARC-Message: https://marc.info/?l=cfe-commits&m=136494006615962 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--nextPart5139700.spAagI2v9v" This is a multi-part message in MIME format. --nextPart5139700.spAagI2v9v Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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 --nextPart5139700.spAagI2v9v Content-Disposition: attachment; filename="0001-Fix-assert-in-DiffTemplate-when-default-template-arg.patch" Content-Transfer-Encoding: 7Bit Content-Type: text/x-patch; charset="UTF-8"; name="0001-Fix-assert-in-DiffTemplate-when-default-template-arg.patch" From f51c329dc66361209381799f4829a3366254631b Mon Sep 17 00:00:00 2001 From: Olivier Goffart 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 struct Trait { enum { V }; }; template ::V> struct K { }; int main() { K f; K 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(FromExpr); - FromValueDecl = cast(DRE->getDecl()); - } - if (!HasToValueDecl && ToExpr) { - DeclRefExpr *DRE = cast(ToExpr); - ToValueDecl = cast(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(ArgExpr)->getDecl(); + default: + assert(0 && "Unexpected template argument kind"); + } + return cast(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 struct Trait { + enum { V = 40 }; + typedef int Ty; + static int I; + }; + int other; + + template ::V > struct A {}; + template ::Ty > struct B {}; + template ::I > struct C {}; + + void test() { + + A a1; + A a2; + A a3; + a1 = a2; + // CHECK-ELIDE-NOTREE: no viable overloaded '=' + // CHECK-ELIDE-NOTREE: no known conversion from 'A' to 'A' + 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' to 'A' + + B b1; + B b2; + B b3; + b1 = b2; + // CHECK-ELIDE-NOTREE: no viable overloaded '=' + // CHECK-ELIDE-NOTREE: no known conversion from 'B::Ty>' to 'B' + b3 = b1; + // CHECK-ELIDE-NOTREE: no viable overloaded '=' + // CHECK-ELIDE-NOTREE: no known conversion from 'B<[...], (default) Trait::Ty>' to 'B<[...], char>' + b2 = b3; + // CHECK-ELIDE-NOTREE: no viable overloaded '=' + // CHECK-ELIDE-NOTREE: no known conversion from 'B' to 'B' + + C c1; + C c2; + C c3; + c1 = c2; + // CHECK-ELIDE-NOTREE: no viable overloaded '=' + // CHECK-ELIDE-NOTREE: no known conversion from 'C' to 'C' + 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' to 'C' + } +} + // 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 --nextPart5139700.spAagI2v9v Content-Disposition: attachment; filename="0002-Fix-crash-in-TemplateDiff-when-comparing-variadic-te.patch" Content-Transfer-Encoding: 7Bit Content-Type: text/x-patch; charset="UTF-8"; name="0002-Fix-crash-in-TemplateDiff-when-comparing-variadic-te.patch" From 1f4e52c914f9cc154de90d2c18198c98f00ffed0 Mon Sep 17 00:00:00 2001 From: Olivier Goffart 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 struct A {}; + template struct B {}; + template 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 b2; + B 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' + + C<> c1; + C c2; + C 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' + } +} + // 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 --nextPart5139700.spAagI2v9v Content-Disposition: attachment; filename="0003-Fix-crash-in-DiffTemplate.patch" Content-Transfer-Encoding: 7Bit Content-Type: text/x-patch; charset="UTF-8"; name="0003-Fix-crash-in-DiffTemplate.patch" From 194a0a92571a42e72e4f82743a1deab99d50be87 Mon Sep 17 00:00:00 2001 From: Olivier Goffart 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' + B b4 = b1; + // CHECK-ELIDE-NOTREE: no viable conversion from 'B<[...], (no argument), (no argument)>' to 'B<[...], i2, i3>' + B b5 = b1; + // CHECK-ELIDE-NOTREE: no viable conversion from 'B<(default) i1, (no argument)>' to 'B' + C<> c1; C c2; -- 1.7.12.1 --nextPart5139700.spAagI2v9v Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits --nextPart5139700.spAagI2v9v--