Reapply "Fix crash on switch conditions of non-integer types in templates"

Melanie Blower · Melanie Blower · commit 759948467ea3 · 2019-11-08T10:17:06.000-08:00
This patch reapplies commit 7694582. 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
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
@@ -103,6 +103,8 @@ struct S {
   static constexpr T t = 0x8000;
   std::string s;
   void f(char c) { s += c | static_cast<int>(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<int>(t)); } 
 };
 
 template S<int>;
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
@@ -233,7 +233,7 @@ struct a {
 template <class>
 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
@@ -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<FieldDecl>(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<CXXRecordDecl>(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
@@ -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<RecordType>()->getDecl();
diff --git 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
@@ -273,9 +273,6 @@ namespace PR10187 {
       }
       int e[10];
     };
-    void g() {
-      S<int>().f(); // expected-note {{here}}
-    }
   }
 
   namespace A2 {
diff --git 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 <Enum v> 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
@@ -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
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct NOT_AN_INTEGRAL_TYPE {};
+
+template <typename T>
+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;
+    }
+  }
+};

Original file line number	Diff line number	Diff line change
`@@ -273,9 +273,6 @@ namespace PR10187 {`
`273`	`273`	`}`
`274`	`274`	`int e[10];`
`275`	`275`	`};`
`276`		`- void g() {`
`277`		`- S<int>().f(); // expected-note {{here}}`
`278`		`- }`
`279`	`276`	`}`
`280`	`277`
`281`	`278`	`namespace A2 {`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,4 @@`
`1`	`1`	`// RUN: %clang_cc1 -fsyntax-only -verify %s`
`2`		`-// expected-no-diagnostics`
`3`	`2`
`4`	`3`	`enum Enum { val = 1 };`
`5`	`4`	`template <Enum v> struct C {`
`@@ -31,7 +30,7 @@ namespace rdar8020920 {`
`31`	`30`	`unsigned long long bitfield : e0;`
`32`	`31`
`33`	`32`	`void f(int j) {`
`34`		`- bitfield + j;`
	`33`	`+ bitfield + j; // expected-warning {{expression result unused}}`
`35`	`34`	`}`
`36`	`35`	`};`
`37`	`36`	`}`
Original file line number	Diff line number	Diff line change
`@@ -156,7 +156,7 @@ namespace test6 {`
`156`	`156`	`void get(B **ptr) {`
`157`	`157`	`// It's okay if at some point we figure out how to diagnose this`
`158`	`158`	`// at instantiation time.`
`159`		`- *ptr = field;`
	`159`	`+ ptr = field; // expected-error {{assigning to 'test6::B ' from incompatible type 'test6::A *}}`
`160`	`160`	`}`
`161`	`161`	`};`
`162`	`162`	`}`