[clang] 878a24e - Reapply "Fix crash on switch conditions of non-integer types in templates"
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 11 11:34:44 PST 2019
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:
template<int b>
class a {
int c : b;
void f() {
if (c)
;
}
};
Please take a look.
On Wed, Dec 4, 2019 at 12:39 AM Elizabeth Andrews via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
>
> Author: Elizabeth Andrews
> Date: 2019-12-03T15:27:19-08:00
> New Revision: 878a24ee244a24c39d1c57e9af2e88c621f7cce9
>
> URL:
> https://github.com/llvm/llvm-project/commit/878a24ee244a24c39d1c57e9af2e88c621f7cce9
> DIFF:
> https://github.com/llvm/llvm-project/commit/878a24ee244a24c39d1c57e9af2e88c621f7cce9.diff
>
> LOG: Reapply "Fix crash on switch conditions of non-integer types in
> templates"
>
> This patch reapplies commit 759948467ea. Patch was reverted due to a
> clang-tidy test fail on Windows. The test has been modified. There
> are no additional code changes.
>
> Patch was tested with ninja check-all on Windows and Linux.
>
> 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.
>
> Added:
> clang/test/SemaTemplate/non-integral-switch-cond.cpp
>
> Modified:
>
> clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp
> clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
> clang/lib/AST/Expr.cpp
> clang/lib/Sema/SemaChecking.cpp
> clang/test/SemaCXX/constant-expression-cxx2a.cpp
> clang/test/SemaTemplate/dependent-names.cpp
> clang/test/SemaTemplate/enum-argument.cpp
> clang/test/SemaTemplate/member-access-expr.cpp
>
> Removed:
>
>
>
>
> ################################################################################
> 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..2c288e0bbddf 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
> @@ -1,4 +1,4 @@
> -// RUN: %check_clang_tidy %s bugprone-string-integer-assignment %t
> +// RUN: %check_clang_tidy %s bugprone-string-integer-assignment %t -- --
> -fno-delayed-template-parsing
>
> namespace std {
> template<typename T>
> @@ -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
> 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>
> 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 322b3a7fa740..a73531ad5fad 100644
> --- a/clang/lib/AST/Expr.cpp
> +++ b/clang/lib/AST/Expr.cpp
> @@ -1678,6 +1678,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
> index ed42833531d4..825e0faa3037 100644
> --- a/clang/lib/Sema/SemaChecking.cpp
> +++ b/clang/lib/Sema/SemaChecking.cpp
> @@ -14706,6 +14706,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
> 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<int>().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 <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
> 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 <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;
> + }
> + }
> +};
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191211/36980a88/attachment-0001.html>
More information about the cfe-commits
mailing list