<div dir="ltr">Any news here? The problem is still there.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Dec 13, 2019 at 12:33 AM Andrews, Elizabeth <<a href="mailto:elizabeth.andrews@intel.com">elizabeth.andrews@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div lang="EN-US">
<div class="gmail-m_3246134216893787678WordSection1">
<p class="MsoNormal">Hi Alexander, <u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">I am debugging this now but I assume the checks for the if condition were skipped before this commit (therefore no crash) because ‘c’ was set as type dependent. The checks are probably run now since c’s type dependency changed with this
patch. The checks might need to be guarded better, but I am not sure. I will take a look and get back to you ASAP.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Elizabeth<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><a name="m_3246134216893787678______replyseparator"></a><b>From:</b> Alexander Kornienko <<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>>
<br>
<b>Sent:</b> Wednesday, December 11, 2019 2:35 PM<br>
<b>To:</b> Andrews, Elizabeth <<a href="mailto:elizabeth.andrews@intel.com" target="_blank">elizabeth.andrews@intel.com</a>>; Elizabeth Andrews <<a href="mailto:llvmlistbot@llvm.org" target="_blank">llvmlistbot@llvm.org</a>><br>
<b>Cc:</b> cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>><br>
<b>Subject:</b> Re: [clang] 878a24e - Reapply "Fix crash on switch conditions of non-integer types in templates"<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">After this commit clang started generating assertion failures on valid code. There are tons of instances in our codebase. Here's a reduced test case:<u></u><u></u></p>
</div>
<p class="MsoNormal">template<int b><br>
class a {<br>
int c : b;<br>
void f() {<br>
if (c)<br>
;<br>
}<br>
};<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Please take a look.<u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">On Wed, Dec 4, 2019 at 12:39 AM Elizabeth Andrews via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal"><br>
Author: Elizabeth Andrews<br>
Date: 2019-12-03T15:27:19-08:00<br>
New Revision: 878a24ee244a24c39d1c57e9af2e88c621f7cce9<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/878a24ee244a24c39d1c57e9af2e88c621f7cce9" target="_blank">
https://github.com/llvm/llvm-project/commit/878a24ee244a24c39d1c57e9af2e88c621f7cce9</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/878a24ee244a24c39d1c57e9af2e88c621f7cce9.diff" target="_blank">
https://github.com/llvm/llvm-project/commit/878a24ee244a24c39d1c57e9af2e88c621f7cce9.diff</a><br>
<br>
LOG: Reapply "Fix crash on switch conditions of non-integer types in templates"<br>
<br>
This patch reapplies commit 759948467ea. Patch was reverted due to a<br>
clang-tidy test fail on Windows. The test has been modified. There<br>
are no additional code changes.<br>
<br>
Patch was tested with ninja check-all on Windows and Linux.<br>
<br>
Summary of code changes:<br>
<br>
Clang currently crashes for switch statements inside a template when the<br>
condition is a non-integer field member because contextual implicit<br>
conversion is skipped when parsing the condition. This conversion is<br>
however later checked in an assert when the case statement is handled.<br>
The conversion is skipped when parsing the condition because<br>
the field member is set as type-dependent based on its containing class.<br>
This patch sets the type dependency based on the field's type instead.<br>
<br>
This patch fixes Bug 40982.<br>
<br>
Added: <br>
clang/test/SemaTemplate/non-integral-switch-cond.cpp<br>
<br>
Modified: <br>
clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp<br>
clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp<br>
clang/lib/AST/Expr.cpp<br>
clang/lib/Sema/SemaChecking.cpp<br>
clang/test/SemaCXX/constant-expression-cxx2a.cpp<br>
clang/test/SemaTemplate/dependent-names.cpp<br>
clang/test/SemaTemplate/enum-argument.cpp<br>
clang/test/SemaTemplate/member-access-expr.cpp<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
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<br>
index 18fe5ef4e5c2..2c288e0bbddf 100644<br>
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp<br>
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp<br>
@@ -1,4 +1,4 @@<br>
-// RUN: %check_clang_tidy %s bugprone-string-integer-assignment %t<br>
+// RUN: %check_clang_tidy %s bugprone-string-integer-assignment %t -- -- -fno-delayed-template-parsing<br>
<br>
namespace std {<br>
template<typename T><br>
@@ -103,6 +103,8 @@ struct S {<br>
static constexpr T t = 0x8000;<br>
std::string s;<br>
void f(char c) { s += c | static_cast<int>(t); }<br>
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: an integer is interpreted as a chara<br>
+ // CHECK-FIXES: {{^}} void f(char c) { s += std::to_string(c | static_cast<int>(t)); }
<br>
};<br>
<br>
template S<int>;<br>
<br>
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<br>
index 119eff67318e..8e546b44ab74 100644<br>
--- a/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp<br>
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp<br>
@@ -233,7 +233,7 @@ struct a {<br>
template <class><br>
class d {<br>
a e;<br>
- void f() { e.b(); }<br>
+ void f() { e.b(0); }<br>
};<br>
} // namespace<br>
} // namespace PR38055<br>
<br>
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp<br>
index 322b3a7fa740..a73531ad5fad 100644<br>
--- a/clang/lib/AST/Expr.cpp<br>
+++ b/clang/lib/AST/Expr.cpp<br>
@@ -1678,6 +1678,15 @@ MemberExpr *MemberExpr::Create(<br>
MemberExpr *E = new (Mem) MemberExpr(Base, IsArrow, OperatorLoc, MemberDecl,<br>
NameInfo, T, VK, OK, NOUR);<br>
<br>
+ if (FieldDecl *Field = dyn_cast<FieldDecl>(MemberDecl)) {<br>
+ DeclContext *DC = MemberDecl->getDeclContext();<br>
+ // dyn_cast_or_null is used to handle objC variables which do not<br>
+ // have a declaration context.<br>
+ CXXRecordDecl *RD = dyn_cast_or_null<CXXRecordDecl>(DC);<br>
+ if (RD && RD->isDependentContext() && RD->isCurrentInstantiation(DC))<br>
+ E->setTypeDependent(T->isDependentType());<br>
+ }<br>
+<br>
if (HasQualOrFound) {<br>
// FIXME: Wrong. We should be looking at the member declaration we found.<br>
if (QualifierLoc && QualifierLoc.getNestedNameSpecifier()->isDependent()) {<br>
<br>
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp<br>
index ed42833531d4..825e0faa3037 100644<br>
--- a/clang/lib/Sema/SemaChecking.cpp<br>
+++ b/clang/lib/Sema/SemaChecking.cpp<br>
@@ -14706,6 +14706,8 @@ void Sema::RefersToMemberWithReducedAlignment(<br>
bool AnyIsPacked = false;<br>
do {<br>
QualType BaseType = ME->getBase()->getType();<br>
+ if (BaseType->isDependentType())<br>
+ return;<br>
if (ME->isArrow())<br>
BaseType = BaseType->getPointeeType();<br>
RecordDecl *RD = BaseType->castAs<RecordType>()->getDecl();<br>
<br>
diff --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp b/clang/test/SemaCXX/constant-expression-cxx2a.cpp<br>
index 8db705dcdc67..c2e443b9bec1 100644<br>
--- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp<br>
+++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp<br>
@@ -18,6 +18,7 @@ namespace std {<br>
[[nodiscard]] void *operator new(std::size_t, std::align_val_t, const std::nothrow_t&) noexcept;<br>
[[nodiscard]] void *operator new[](std::size_t, const std::nothrow_t&) noexcept;<br>
[[nodiscard]] void *operator new[](std::size_t, std::align_val_t, const std::nothrow_t&) noexcept;<br>
+[[nodiscard]] void *operator new[](std::size_t, std::align_val_t);<br>
void operator delete(void*, const std::nothrow_t&) noexcept;<br>
void operator delete(void*, std::align_val_t, const std::nothrow_t&) noexcept;<br>
void operator delete[](void*, const std::nothrow_t&) noexcept;<br>
@@ -1050,7 +1051,7 @@ namespace dynamic_alloc {<br>
// Ensure that we don't try to evaluate these for overflow and crash. These<br>
// are all value-dependent expressions.<br>
p = new char[n];<br>
- p = new (n) char[n];<br>
+ p = new ((std::align_val_t)n) char[n];<br>
p = new char(n);<br>
}<br>
}<br>
<br>
diff --git a/clang/test/SemaTemplate/dependent-names.cpp b/clang/test/SemaTemplate/dependent-names.cpp<br>
index 67ef238083f0..a8de159a1d46 100644<br>
--- a/clang/test/SemaTemplate/dependent-names.cpp<br>
+++ b/clang/test/SemaTemplate/dependent-names.cpp<br>
@@ -273,9 +273,6 @@ namespace PR10187 {<br>
}<br>
int e[10];<br>
};<br>
- void g() {<br>
- S<int>().f(); // expected-note {{here}}<br>
- }<br>
}<br>
<br>
namespace A2 {<br>
<br>
diff --git a/clang/test/SemaTemplate/enum-argument.cpp b/clang/test/SemaTemplate/enum-argument.cpp<br>
index 7ff419613990..a79ed8403e9f 100644<br>
--- a/clang/test/SemaTemplate/enum-argument.cpp<br>
+++ b/clang/test/SemaTemplate/enum-argument.cpp<br>
@@ -1,5 +1,4 @@<br>
// RUN: %clang_cc1 -fsyntax-only -verify %s<br>
-// expected-no-diagnostics<br>
<br>
enum Enum { val = 1 };<br>
template <Enum v> struct C {<br>
@@ -31,7 +30,7 @@ namespace rdar8020920 {<br>
unsigned long long bitfield : e0;<br>
<br>
void f(int j) {<br>
- bitfield + j;<br>
+ bitfield + j; // expected-warning {{expression result unused}}<br>
}<br>
};<br>
}<br>
<br>
diff --git a/clang/test/SemaTemplate/member-access-expr.cpp b/clang/test/SemaTemplate/member-access-expr.cpp<br>
index 8dba2e68d656..ef10d72a0ef8 100644<br>
--- a/clang/test/SemaTemplate/member-access-expr.cpp<br>
+++ b/clang/test/SemaTemplate/member-access-expr.cpp<br>
@@ -156,7 +156,7 @@ namespace test6 {<br>
void get(B **ptr) {<br>
// It's okay if at some point we figure out how to diagnose this<br>
// at instantiation time.<br>
- *ptr = field;<br>
+ *ptr = field; // expected-error {{assigning to 'test6::B *' from incompatible type 'test6::A *}}<br>
}<br>
};<br>
}<br>
<br>
diff --git a/clang/test/SemaTemplate/non-integral-switch-cond.cpp b/clang/test/SemaTemplate/non-integral-switch-cond.cpp<br>
new file mode 100644<br>
index 000000000000..23c8e0ef8d4e<br>
--- /dev/null<br>
+++ b/clang/test/SemaTemplate/non-integral-switch-cond.cpp<br>
@@ -0,0 +1,14 @@<br>
+// RUN: %clang_cc1 -fsyntax-only -verify %s<br>
+<br>
+struct NOT_AN_INTEGRAL_TYPE {};<br>
+<br>
+template <typename T><br>
+struct foo {<br>
+ NOT_AN_INTEGRAL_TYPE Bad;<br>
+ void run() {<br>
+ switch (Bad) { // expected-error {{statement requires expression of integer type ('NOT_AN_INTEGRAL_TYPE' invalid)}}<br>
+ case 0:<br>
+ break;<br>
+ }<br>
+ }<br>
+};<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><u></u><u></u></p>
</blockquote>
</div>
</div>
</div>
</blockquote></div>