From 759948467ea3181615d44d80f74ffeb260180fd0 Mon Sep 17 00:00:00 2001 From: Melanie Blower Date: Fri, 8 Nov 2019 10:14:51 -0800 Subject: [PATCH] Reapply "Fix crash on switch conditions of non-integer types in templates" This patch reapplies commit 76945821b9cad3. The first version broke buildbots due to clang-tidy test fails. The fails are because some errors in templates are now diagnosed earlier (does not wait till instantiation). I have modified the tests to add checks for these diagnostics/prevent these diagnostics. There are no additional code changes. Summary of code changes: Clang currently crashes for switch statements inside a template when the condition is a non-integer field member because contextual implicit conversion is skipped when parsing the condition. This conversion is however later checked in an assert when the case statement is handled. The conversion is skipped when parsing the condition because the field member is set as type-dependent based on its containing class. This patch sets the type dependency based on the field's type instead. This patch fixes Bug 40982. Reviewers: rnk, gribozavr2 Patch by: Elizabeth Andrews (eandrews) Differential revision: https://reviews.llvm.org/D69950 --- .../bugprone-string-integer-assignment.cpp | 2 ++ .../clang-tidy/checkers/misc-unused-parameters.cpp | 2 +- clang/lib/AST/Expr.cpp | 9 +++++++++ clang/lib/Sema/SemaChecking.cpp | 2 ++ clang/test/SemaCXX/constant-expression-cxx2a.cpp | 3 ++- clang/test/SemaTemplate/dependent-names.cpp | 3 --- clang/test/SemaTemplate/enum-argument.cpp | 3 +-- clang/test/SemaTemplate/member-access-expr.cpp | 2 +- .../test/SemaTemplate/non-integral-switch-cond.cpp | 14 ++++++++++++++ 9 files changed, 32 insertions(+), 8 deletions(-) create mode 100644 clang/test/SemaTemplate/non-integral-switch-cond.cpp diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp index 18fe5ef4e5c2..79d41ef77c80 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp @@ -103,6 +103,8 @@ struct S { static constexpr T t = 0x8000; std::string s; void f(char c) { s += c | static_cast(t); } + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: an integer is interpreted as a chara + // CHECK-FIXES: {{^}} void f(char c) { s += std::to_string(c | static_cast(t)); } }; template S; diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp index 119eff67318e..8e546b44ab74 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp @@ -233,7 +233,7 @@ struct a { template class d { a e; - void f() { e.b(); } + void f() { e.b(0); } }; } // namespace } // namespace PR38055 diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 3438c3aadc6b..00ba329642c8 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -1675,6 +1675,15 @@ MemberExpr *MemberExpr::Create( MemberExpr *E = new (Mem) MemberExpr(Base, IsArrow, OperatorLoc, MemberDecl, NameInfo, T, VK, OK, NOUR); + if (FieldDecl *Field = dyn_cast(MemberDecl)) { + DeclContext *DC = MemberDecl->getDeclContext(); + // dyn_cast_or_null is used to handle objC variables which do not + // have a declaration context. + CXXRecordDecl *RD = dyn_cast_or_null(DC); + if (RD && RD->isDependentContext() && RD->isCurrentInstantiation(DC)) + E->setTypeDependent(T->isDependentType()); + } + if (HasQualOrFound) { // FIXME: Wrong. We should be looking at the member declaration we found. if (QualifierLoc && QualifierLoc.getNestedNameSpecifier()->isDependent()) { diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 8322a9bf1477..45a3a7f5b00d 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -14682,6 +14682,8 @@ void Sema::RefersToMemberWithReducedAlignment( bool AnyIsPacked = false; do { QualType BaseType = ME->getBase()->getType(); + if (BaseType->isDependentType()) + return; if (ME->isArrow()) BaseType = BaseType->getPointeeType(); RecordDecl *RD = BaseType->castAs()->getDecl(); diff --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp b/clang/test/SemaCXX/constant-expression-cxx2a.cpp index 8db705dcdc67..c2e443b9bec1 100644 --- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp +++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp @@ -18,6 +18,7 @@ namespace std { [[nodiscard]] void *operator new(std::size_t, std::align_val_t, const std::nothrow_t&) noexcept; [[nodiscard]] void *operator new[](std::size_t, const std::nothrow_t&) noexcept; [[nodiscard]] void *operator new[](std::size_t, std::align_val_t, const std::nothrow_t&) noexcept; +[[nodiscard]] void *operator new[](std::size_t, std::align_val_t); void operator delete(void*, const std::nothrow_t&) noexcept; void operator delete(void*, std::align_val_t, const std::nothrow_t&) noexcept; void operator delete[](void*, const std::nothrow_t&) noexcept; @@ -1050,7 +1051,7 @@ namespace dynamic_alloc { // Ensure that we don't try to evaluate these for overflow and crash. These // are all value-dependent expressions. p = new char[n]; - p = new (n) char[n]; + p = new ((std::align_val_t)n) char[n]; p = new char(n); } } diff --git a/clang/test/SemaTemplate/dependent-names.cpp b/clang/test/SemaTemplate/dependent-names.cpp index 67ef238083f0..a8de159a1d46 100644 --- a/clang/test/SemaTemplate/dependent-names.cpp +++ b/clang/test/SemaTemplate/dependent-names.cpp @@ -273,9 +273,6 @@ namespace PR10187 { } int e[10]; }; - void g() { - S().f(); // expected-note {{here}} - } } namespace A2 { diff --git a/clang/test/SemaTemplate/enum-argument.cpp b/clang/test/SemaTemplate/enum-argument.cpp index 7ff419613990..a79ed8403e9f 100644 --- a/clang/test/SemaTemplate/enum-argument.cpp +++ b/clang/test/SemaTemplate/enum-argument.cpp @@ -1,5 +1,4 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s -// expected-no-diagnostics enum Enum { val = 1 }; template struct C { @@ -31,7 +30,7 @@ namespace rdar8020920 { unsigned long long bitfield : e0; void f(int j) { - bitfield + j; + bitfield + j; // expected-warning {{expression result unused}} } }; } diff --git a/clang/test/SemaTemplate/member-access-expr.cpp b/clang/test/SemaTemplate/member-access-expr.cpp index 8dba2e68d656..ef10d72a0ef8 100644 --- a/clang/test/SemaTemplate/member-access-expr.cpp +++ b/clang/test/SemaTemplate/member-access-expr.cpp @@ -156,7 +156,7 @@ namespace test6 { void get(B **ptr) { // It's okay if at some point we figure out how to diagnose this // at instantiation time. - *ptr = field; + *ptr = field; // expected-error {{assigning to 'test6::B *' from incompatible type 'test6::A *}} } }; } diff --git a/clang/test/SemaTemplate/non-integral-switch-cond.cpp b/clang/test/SemaTemplate/non-integral-switch-cond.cpp new file mode 100644 index 000000000000..23c8e0ef8d4e --- /dev/null +++ b/clang/test/SemaTemplate/non-integral-switch-cond.cpp @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +struct NOT_AN_INTEGRAL_TYPE {}; + +template +struct foo { + NOT_AN_INTEGRAL_TYPE Bad; + void run() { + switch (Bad) { // expected-error {{statement requires expression of integer type ('NOT_AN_INTEGRAL_TYPE' invalid)}} + case 0: + break; + } + } +};