=?utf-8?q?André?= Brand <andre.br...@mailbox.org>, =?utf-8?q?André?= Brand <andre.br...@mailbox.org>,thebrandre <andre.br...@mailbox.org>, =?utf-8?q?André?= Brand <andre.br...@mailbox.org> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/121...@github.com>
https://github.com/thebrandre updated https://github.com/llvm/llvm-project/pull/121039 >From 00dad7d3f3455e67d4a64c852d581fa5c17ff27c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Brand?= <andre.br...@mailbox.org> Date: Sun, 5 Jan 2025 14:48:40 +0100 Subject: [PATCH 1/5] [clang] Fix missing promotion type for opaque enum declarations in class templates Repeat the steps in `Sema::ActOnTag` for the promotion type after type substitution of the underlying type. This is required in the case of an *opaque-enum-declaration* (see [dcl.enum]). If, instead, a full *enum-specifier* (subsequent curly braces) is provided, `Sema::ActOnEnumBody` is re-invoked on template instantiation overriding the promotion type and hiding the issue. Note that, in contrast to `Sema::ActOnEnumBody`, `Sema::ActOnTag` is *not* called again for the instantiated enumeration type. Fixes #117960. --- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index e058afe81da589..87342103aa15af 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -1571,6 +1571,23 @@ Decl *TemplateDeclInstantiator::VisitEnumDecl(EnumDecl *D) { Enum->setIntegerType(SemaRef.Context.IntTy); else Enum->setIntegerTypeSourceInfo(NewTI); + + // The following lines are relevant for opaque-enum-declarations. + // If users declare a full enum-specifier, the promotion type is reset + // again when parsing the (potentially empty) enumerator-list. + // We implement the same logic from C++11 [conv.prom] p4 here but only + // after template specialization [temp.spec.general] because it requires + // the concrete type. + // Note that (1) this also correctly handles the non-dependent case, + // (2) we set the promotion type for scoped enumerations to ensure + // consistency with declarations outside of templates, (3) we guarantee a + // valid promotion even if the user provided an invalid underlying type + // (fallback to "default int"). + QualType UnderlyingType = Enum->getIntegerType(); + Enum->setPromotionType( + SemaRef.Context.isPromotableIntegerType(UnderlyingType) + ? SemaRef.Context.getPromotedIntegerType(UnderlyingType) + : UnderlyingType); } else { assert(!D->getIntegerType()->isDependentType() && "Dependent type without type source info"); >From 3ed6a0ce6ad37a171293d57e98537984cc3af4f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Brand?= <andre.br...@mailbox.org> Date: Sun, 5 Jan 2025 14:48:44 +0100 Subject: [PATCH 2/5] [clang] Add tests for implicit integer conversion of enums declared in class templates These tests are variants of the crash reported in GitHub issue #117960, in which an opaque-enum-declaration in a class template broke basic assumptions during parsing of arithmetic expressions due to missing promotion types of instances of EnumDecl. --- ...que-enum-declaration-in-class-template.cpp | 214 ++++++++++++++++++ 1 file changed, 214 insertions(+) create mode 100644 clang/test/SemaCXX/opaque-enum-declaration-in-class-template.cpp diff --git a/clang/test/SemaCXX/opaque-enum-declaration-in-class-template.cpp b/clang/test/SemaCXX/opaque-enum-declaration-in-class-template.cpp new file mode 100644 index 00000000000000..7101a153c6ebb7 --- /dev/null +++ b/clang/test/SemaCXX/opaque-enum-declaration-in-class-template.cpp @@ -0,0 +1,214 @@ +// RUN: %clang_cc1 -std=c++11 -Wredeclared-class-member -Wconstant-conversion -Wdeprecated-declarations -Wc++11-narrowing -fsyntax-only %s -verify +// RUN: %clang_cc1 -std=c++14 -Wredeclared-class-member -Wconstant-conversion -Wdeprecated-declarations -Wc++11-narrowing -fsyntax-only %s -verify +// RUN: %clang_cc1 -std=c++20 -Wredeclared-class-member -Wconstant-conversion -Wdeprecated-declarations -Wc++11-narrowing -fsyntax-only %s -verify + +// Test that opaque-enum-declarations are handled correctly w.r.t integral promotions. +// The key sections in the C++11 standard are: +// C++11 [dcl.enum]p3: An enumeration declared by an opaque-enum-declaration +// has a fixed underlying type and is a complete type. +// C++11 [conv.prom]: A prvalue of an unscoped enumeration type whose underlying type +// is fixed ([dcl.enum]) can be converted to a prvalue of its underlying type. + +// This program causes clang 19 and earlier to crash because +// EnumDecl::PromotionType has not been set on the instantiated enum. +// See GitHub Issue #117960. +namespace Issue117960 { +template <typename T> +struct A { + enum E : T; +}; + +int b = A<int>::E{} + 0; +} + + +namespace test { +template <typename T1, typename T2> +struct IsSame { + static constexpr bool check() { return false; } +}; + +template <typename T> +struct IsSame<T, T> { + static constexpr bool check() { return true; } +}; +} // namespace test + + +template <typename T> +struct S1 { + enum E : T; +}; +// checks if EnumDecl::PromotionType is set +int X1 = S1<int>::E{} + 0; +int Y1 = S1<unsigned>::E{} + 0; +static_assert(test::IsSame<decltype(S1<int>::E{}+0), int>::check(), ""); +static_assert(test::IsSame<decltype(S1<unsigned>::E{}+0), unsigned>::check(), ""); +char Z1 = S1<unsigned>::E(-1) + 0; // expected-warning{{implicit conversion from 'unsigned int' to 'char'}} + +template <typename Traits> +struct S2 { + enum E : typename Traits::IntegerType; +}; + +template <typename T> +struct Traits { + typedef T IntegerType; +}; + +int X2 = S2<Traits<int>>::E{} + 0; +int Y2 = S2<Traits<unsigned>>::E{} + 0; +static_assert(test::IsSame<decltype(S2<Traits<int>>::E{}+0), int>::check(), ""); +static_assert(test::IsSame<decltype(S2<Traits<unsigned>>::E{}+0), unsigned>::check(), ""); +// C++11 [conv.prom]p4: +// A prvalue of an unscoped enumeration type whose underlying type is fixed can be converted to a +// prvalue of its underlying type. Moreover, if integral promotion can be applied to its underlying type, a +// prvalue of an unscoped enumeration type whose underlying type is fixed can also be converted to a prvalue +// of the promoted underlying type. +static_assert(test::IsSame<decltype(S2<Traits<char>>::E{}+char(0)), int>::check(), ""); + + +template <typename T> +struct S3 { + enum E : unsigned; +}; + +int X3 = S3<float>::E{} + 0; + +// fails in clang 19 and earlier (see the discussion on GitHub Issue #117960): +static_assert(test::IsSame<decltype(S3<float>::E{}+0), unsigned>::check(), ""); + +template <typename T> +struct S4 { + enum E1 : char; + enum E2 : T; +}; + +int X4 = S4<char>::E1{} + '\0'; +int Y4 = S4<char>::E2{} + '\0'; + +template <typename T> +struct S5 { + enum class E1 : char; + enum class E2 : T; +}; + +int X5 = S5<char>::E1{} + '\0'; // expected-error{{invalid operands to binary expression}} +int Y5 = S5<char>::E2{} + '\0'; // expected-error{{invalid operands to binary expression}} + + +template <typename T> +struct S6 { + enum E1 : T; + enum E2 : E1; // expected-error{{invalid underlying type}} +}; + +template struct S6<int>; // expected-note{{in instantiation of template class 'S6<int>' requested here}} + + +template <typename T> +struct S7 { + enum E : T; + enum E : T { X, Y, Z }; // expected-note{{previous declaration is here}} + enum E : T; // expected-warning{{class member cannot be redeclared}} +}; + +template struct S7<int>; + +template <typename T> +struct S8 { + enum E : char; + enum E : char { X, Y, Z }; // expected-note{{previous declaration is here}} + enum E : char; // expected-warning{{class member cannot be redeclared}} +}; + +template struct S8<float>; + +template <typename T> +struct S9 { + enum class E1 : T; + enum class E1 : T { X, Y, Z }; // expected-note{{previous declaration is here}} + enum class E1 : T; // expected-warning{{class member cannot be redeclared}} + enum class E2 : char; + enum class E2 : char { X, Y, Z }; // expected-note{{previous declaration is here}} + enum class E2 : char; // expected-warning{{class member cannot be redeclared}} +}; + +template struct S9<int>; + +#if defined(__cplusplus) && __cplusplus >= 201402L +template <typename T> +struct S10 { + enum [[deprecated("for reasons")]] E : T; // expected-note{{explicitly marked deprecated here}} +}; + +int X10 = S10<int>::E{} + 0; // expected-warning{{deprecated: for reasons}} +#endif + +template <typename T> +struct S11 {}; + +template <> +struct S11<unsigned> { + enum E : unsigned; +}; + +unsigned X11 = S11<unsigned>::E{} + 0u; + +#if defined(__cplusplus) && __cplusplus >= 201402L +template <typename T> +struct S12 { + enum [[deprecated("for reasons")]] E1 : T; // expected-note{{explicitly marked deprecated here}} + enum [[deprecated("for reasons")]] E2 : T; +}; + +template <> +struct S12<float> { + enum E1 : unsigned; + enum E2 : unsigned; +}; + +unsigned X12 = S12<float>::E1{} + 0u; +unsigned Y12 = S12<float>::E2{} + 0u; +int Z12 = S12<int>::E1{} + 0; // expected-warning{{deprecated: for reasons}} +#endif + +template <typename T> +struct S13 { + enum __attribute__((packed)) E { X, Y }; +}; + +static_assert(sizeof(S13<int>::E) == 1, ""); + +template<typename T> +struct S14 { + enum E : float; // expected-error {{invalid underlying type}} +}; + +template<typename T> +struct S15 { + enum E : T; // expected-error {{invalid underlying type}} +}; + +template struct S15<float>; // expected-note {{in instantiation of template class 'S15<float>' requested here}} + + + + +template <typename T> +int f1() { + enum E : T; + return E{} + 0; +} + +int F1 = f1<int>(); + +template <typename T> +int f2() { + struct LocalClass { + enum E : T; + }; + return typename LocalClass::E{} + 0; +} + +int F2 = f2<int>(); >From cfc5eb522e127498e38cf80d017b2489da844661 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Brand?= <andre.br...@mailbox.org> Date: Tue, 7 Jan 2025 19:31:54 +0100 Subject: [PATCH 3/5] [clang] Add release notes for resolving issue #117960 --- clang/docs/ReleaseNotes.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 93915e5db7d131..efe1a0ac6e54fd 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -938,6 +938,9 @@ Miscellaneous Clang Crashes Fixed - Fixed internal assertion firing when a declaration in the implicit global module is found through ADL. (GH#109879) +- Fixed a crash when an unscoped enumeration declared by an opaque-enum-declaration within a class template + with a dependent underlying type is subject to integral promotion. (GH#117960) + OpenACC Specific Changes ------------------------ >From e652f82cd32c9947cd9ef98adab1687133ce25aa Mon Sep 17 00:00:00 2001 From: thebrandre <andre.br...@mailbox.org> Date: Tue, 7 Jan 2025 19:35:49 +0100 Subject: [PATCH 4/5] [clang] Reduce verbosity of comment and add FIXME Co-authored-by: cor3ntin <corentinja...@gmail.com> --- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 87342103aa15af..e489bb829097f9 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -1571,18 +1571,11 @@ Decl *TemplateDeclInstantiator::VisitEnumDecl(EnumDecl *D) { Enum->setIntegerType(SemaRef.Context.IntTy); else Enum->setIntegerTypeSourceInfo(NewTI); - - // The following lines are relevant for opaque-enum-declarations. - // If users declare a full enum-specifier, the promotion type is reset - // again when parsing the (potentially empty) enumerator-list. - // We implement the same logic from C++11 [conv.prom] p4 here but only - // after template specialization [temp.spec.general] because it requires - // the concrete type. - // Note that (1) this also correctly handles the non-dependent case, - // (2) we set the promotion type for scoped enumerations to ensure - // consistency with declarations outside of templates, (3) we guarantee a - // valid promotion even if the user provided an invalid underlying type - // (fallback to "default int"). + // C++23 [conv.prom]p4 + // if integral promotion can be applied to its underlying type, a prvalue of an unscoped enumeration type + // whose underlying type is fixed can also be converted to a prvalue of the promoted underlying type. + // + // FIXME: that logic is already implemented in ActOnEnumBody, factor out into (Re)BuildEnumBody. QualType UnderlyingType = Enum->getIntegerType(); Enum->setPromotionType( SemaRef.Context.isPromotableIntegerType(UnderlyingType) >From a2e922730086d5ed6cece7adedff55c0f606437b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Brand?= <andre.br...@mailbox.org> Date: Tue, 7 Jan 2025 19:44:45 +0100 Subject: [PATCH 5/5] [clang] Reformat code --- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index e489bb829097f9..6a2331e59477a2 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -1571,11 +1571,14 @@ Decl *TemplateDeclInstantiator::VisitEnumDecl(EnumDecl *D) { Enum->setIntegerType(SemaRef.Context.IntTy); else Enum->setIntegerTypeSourceInfo(NewTI); + // C++23 [conv.prom]p4 - // if integral promotion can be applied to its underlying type, a prvalue of an unscoped enumeration type - // whose underlying type is fixed can also be converted to a prvalue of the promoted underlying type. - // - // FIXME: that logic is already implemented in ActOnEnumBody, factor out into (Re)BuildEnumBody. + // if integral promotion can be applied to its underlying type, a prvalue + // of an unscoped enumeration type whose underlying type is fixed can also + // be converted to a prvalue of the promoted underlying type. + // + // FIXME: that logic is already implemented in ActOnEnumBody, factor out + // into (Re)BuildEnumBody. QualType UnderlyingType = Enum->getIntegerType(); Enum->setPromotionType( SemaRef.Context.isPromotableIntegerType(UnderlyingType) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
