Skip to content

Commit

Permalink
Reapply "Fix crash on switch conditions of non-integer types in templ…
Browse files Browse the repository at this point in the history
…ates"

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
  • Loading branch information
Melanie Blower committed Nov 8, 2019
1 parent 7f92d66 commit 7599484
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 8 deletions.
Expand Up @@ -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>;
Expand Down
Expand Up @@ -233,7 +233,7 @@ struct a {
template <class>
class d {
a e;
void f() { e.b(); }
void f() { e.b(0); }
};
} // namespace
} // namespace PR38055
Expand Down
9 changes: 9 additions & 0 deletions clang/lib/AST/Expr.cpp
Expand Up @@ -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()) {
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Sema/SemaChecking.cpp
Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion clang/test/SemaCXX/constant-expression-cxx2a.cpp
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand Down
3 changes: 0 additions & 3 deletions clang/test/SemaTemplate/dependent-names.cpp
Expand Up @@ -273,9 +273,6 @@ namespace PR10187 {
}
int e[10];
};
void g() {
S<int>().f(); // expected-note {{here}}
}
}

namespace A2 {
Expand Down
3 changes: 1 addition & 2 deletions 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 {
Expand Down Expand Up @@ -31,7 +30,7 @@ namespace rdar8020920 {
unsigned long long bitfield : e0;

void f(int j) {
bitfield + j;
bitfield + j; // expected-warning {{expression result unused}}
}
};
}
2 changes: 1 addition & 1 deletion clang/test/SemaTemplate/member-access-expr.cpp
Expand Up @@ -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 *}}
}
};
}
14 changes: 14 additions & 0 deletions 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;
}
}
};

0 comments on commit 7599484

Please sign in to comment.