diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 57a94242c9e61..a90306f2ba6c9 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -310,6 +310,15 @@ C23 Feature Support `WG14 N2975 `_ - Fixed a bug with handling the type operand form of ``typeof`` when it is used to specify a fixed underlying type for an enumeration. #GH146351 +- Fixed a rejects-valid bug where Clang would reject an enumeration with an + ``_Atomic`` underlying type. The underlying type is the non-atomic, + unqualified version of the specified type. Due to the perhaps surprising lack + of atomic behavior, this is diagnosed under + ``-Wunderlying-atomic-qualifier-ignored``, which defaults to an error. This + can be downgraded with ``-Wno-underlying-atomic-qualifier-ignored`` or + ``-Wno-error=underlying-atomic-qualifier-ignored``. Clang now also diagnoses + cv-qualifiers as being ignored, but that warning does not default to an error. + It can be controlled by ``-Wunderlying-cv-qualifier-ignore``. (#GH147736) C11 Feature Support ^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 0b2553d82153c..ef59cf25541b2 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -9364,6 +9364,16 @@ def warn_atomic_implicit_seq_cst : Warning< InGroup>, DefaultIgnore; def err_atomic_unsupported : Error< "atomic types are not supported in '%0'">; +def warn_cv_stripped_in_enum : Warning< + "%enum_select{" + "%Both{'const' and 'volatile' qualifiers}|" + "%Const{'const' qualifier}|" + "%Volatile{'volatile' qualifier}}0 in enumeration underlying type ignored">, + InGroup>; +def warn_atomic_stripped_in_enum : Warning< + "'_Atomic' qualifier ignored; operations involving the enumeration type will " + "be non-atomic">, + InGroup>, DefaultError; def err_overflow_builtin_must_be_int : Error< "operand argument to %select{overflow builtin|checked integer operation}0 " diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 11cbda412667f..d7234e269f645 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -17155,6 +17155,30 @@ bool Sema::CheckEnumUnderlyingType(TypeSourceInfo *TI) { if (T->isDependentType()) return false; + // C++0x 7.2p2: The type-specifier-seq of an enum-base shall name an + // integral type; any cv-qualification is ignored. + // C23 6.7.3.3p5: The underlying type of the enumeration is the unqualified, + // non-atomic version of the type specified by the type specifiers in the + // specifier qualifier list. + // Because of how odd C's rule is, we'll let the user know that operations + // involving the enumeration type will be non-atomic. + if (T->isAtomicType()) + Diag(UnderlyingLoc, diag::warn_atomic_stripped_in_enum); + + Qualifiers Q = T.getQualifiers(); + std::optional QualSelect; + if (Q.hasConst() && Q.hasVolatile()) + QualSelect = diag::CVQualList::Both; + else if (Q.hasConst()) + QualSelect = diag::CVQualList::Const; + else if (Q.hasVolatile()) + QualSelect = diag::CVQualList::Volatile; + + if (QualSelect) + Diag(UnderlyingLoc, diag::warn_cv_stripped_in_enum) << *QualSelect; + + T = T.getAtomicUnqualifiedType(); + // This doesn't use 'isIntegralType' despite the error message mentioning // integral type because isIntegralType would also allow enum types in C. if (const BuiltinType *BT = T->getAs()) @@ -17551,6 +17575,9 @@ Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc, } else if (UnderlyingType.get()) { // C++0x 7.2p2: The type-specifier-seq of an enum-base shall name an // integral type; any cv-qualification is ignored. + // C23 6.7.3.3p5: The underlying type of the enumeration is the + // unqualified, non-atomic version of the type specified by the type + // specifiers in the specifier qualifier list. TypeSourceInfo *TI = nullptr; GetTypeFromParser(UnderlyingType.get(), &TI); EnumUnderlying = TI; @@ -17563,6 +17590,18 @@ Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc, UPPC_FixedUnderlyingType)) EnumUnderlying = Context.IntTy.getTypePtr(); + // If the underlying type is atomic, we need to adjust the type before + // continuing. This only happens in the case we stored a TypeSourceInfo + // into EnumUnderlying because the other cases are error recovery up to + // this point. But because it's not possible to gin up a TypeSourceInfo + // for a non-atomic type from an atomic one, we'll store into the Type + // field instead. FIXME: it would be nice to have an easy way to get a + // derived TypeSourceInfo which strips qualifiers including the weird + // ones like _Atomic where it forms a different type. + if (TypeSourceInfo *TI = dyn_cast(EnumUnderlying); + TI && TI->getType()->isAtomicType()) + EnumUnderlying = TI->getType().getAtomicUnqualifiedType().getTypePtr(); + } else if (Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment()) { // For MSVC ABI compatibility, unfixed enums must use an underlying type // of 'int'. However, if this is an unfixed forward declaration, don't set diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 70a4c159f9805..e2c3cdcd536bc 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -2022,8 +2022,17 @@ Decl *TemplateDeclInstantiator::VisitEnumDecl(EnumDecl *D) { DeclarationName()); if (!NewTI || SemaRef.CheckEnumUnderlyingType(NewTI)) Enum->setIntegerType(SemaRef.Context.IntTy); - else - Enum->setIntegerTypeSourceInfo(NewTI); + else { + // If the underlying type is atomic, we need to adjust the type before + // continuing. See C23 6.7.3.3p5 and Sema::ActOnTag(). FIXME: same as + // within ActOnTag(), it would be nice to have an easy way to get a + // derived TypeSourceInfo which strips qualifiers including the weird + // ones like _Atomic where it forms a different type. + if (NewTI->getType()->isAtomicType()) + Enum->setIntegerType(NewTI->getType().getAtomicUnqualifiedType()); + else + Enum->setIntegerTypeSourceInfo(NewTI); + } // C++23 [conv.prom]p4 // if integral promotion can be applied to its underlying type, a prvalue diff --git a/clang/test/C/C23/n3030.c b/clang/test/C/C23/n3030.c index 17084bbb55f50..94ea7037edd11 100644 --- a/clang/test/C/C23/n3030.c +++ b/clang/test/C/C23/n3030.c @@ -91,3 +91,19 @@ enum e : short f = 0; // expected-error {{non-defining declaration of enumeratio enum g : short { yyy } h = yyy; enum ee2 : typeof ((enum ee3 : short { A })0, (short)0); + +enum not_actually_atomic : _Atomic(short) { // expected-error {{'_Atomic' qualifier ignored; operations involving the enumeration type will be non-atomic}} + Surprise +}; + +enum not_actually_const : const int { // expected-warning {{'const' qualifier in enumeration underlying type ignored}} + SurpriseAgain +}; + +enum not_actually_volatile : volatile int { // expected-warning {{'volatile' qualifier in enumeration underlying type ignored}} + SurpriseOnceMore +}; + +enum not_acually_const_or_volatile : const volatile int { // expected-warning {{'const' and 'volatile' qualifiers in enumeration underlying type ignored}} + WhyTheSurprise +}; diff --git a/clang/test/C/C23/n3030_1.c b/clang/test/C/C23/n3030_1.c new file mode 100644 index 0000000000000..1afc9855767f0 --- /dev/null +++ b/clang/test/C/C23/n3030_1.c @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -std=c23 -Wno-underlying-atomic-qualifier-ignored -ast-dump %s | FileCheck %s + +// The underlying type is the unqualified, non-atomic version of the type +// specified. +enum const_enum : const short { ConstE }; +// CHECK: EnumDecl {{.*}} const_enum 'short' + +// These were previously being diagnosed as invalid underlying types. They +// are valid; the _Atomic is stripped from the underlying type. +enum atomic_enum1 : _Atomic(int) { AtomicE1 }; +// CHECK: EnumDecl {{.*}} atomic_enum1 'int' +enum atomic_enum2 : _Atomic long long { AtomicE2 }; +// CHECK: EnumDecl {{.*}} atomic_enum2 'long long' diff --git a/clang/test/CXX/dcl.dcl/dcl.enum/p2.cpp b/clang/test/CXX/dcl.dcl/dcl.enum/p2.cpp index de826d0570422..7b69358687a2f 100644 --- a/clang/test/CXX/dcl.dcl/dcl.enum/p2.cpp +++ b/clang/test/CXX/dcl.dcl/dcl.enum/p2.cpp @@ -1,6 +1,5 @@ // RUN: %clang_cc1 -std=c++11 -verify %s -// expected-no-diagnostics -enum class E : int const volatile { }; +enum class E : int const volatile { }; // expected-warning {{'const' and 'volatile' qualifiers in enumeration underlying type ignored}} using T = __underlying_type(E); using T = int; diff --git a/clang/test/CodeGen/enum3.c b/clang/test/CodeGen/enum3.c new file mode 100644 index 0000000000000..6878a0bbb94d0 --- /dev/null +++ b/clang/test/CodeGen/enum3.c @@ -0,0 +1,26 @@ +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5 +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-error=underlying-atomic-qualifier-ignored -std=c23 %s -emit-llvm -o - | FileCheck %s + +// Ensure that an "atomic" underlying type has no actual atomic semantics +// because the qualifier is stripped. + +enum E : _Atomic(int) { + Foo +}; + +// CHECK-LABEL: define {{.*}} void @test( +// CHECK-SAME: i32 noundef [[E:%.*]]) #[[ATTR0:[0-9]+]] { +// CHECK-NEXT: [[ENTRY:.*:]] +// CHECK-NEXT: [[E_ADDR:%.*]] = alloca i32 +// CHECK-NEXT: [[X:%.*]] = alloca i32 +// CHECK-NEXT: store i32 [[E]], ptr [[E_ADDR]] +// CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[E_ADDR]] +// CHECK-NEXT: store i32 [[TMP0]], ptr [[X]] +// CHECK-NEXT: store i32 0, ptr [[E_ADDR]] +// CHECK-NEXT: ret void +// +void test(enum E e) { + int x = e; + e = Foo; +} + diff --git a/clang/test/SemaCXX/enum-scoped.cpp b/clang/test/SemaCXX/enum-scoped.cpp index d7b7923430aff..0ce47274979d9 100644 --- a/clang/test/SemaCXX/enum-scoped.cpp +++ b/clang/test/SemaCXX/enum-scoped.cpp @@ -349,3 +349,18 @@ enum class B; A a; B b{a}; // expected-error {{cannot initialize}} } + +namespace GH147736 { +template +struct S { + enum OhBoy : Ty { // expected-error 2 {{'_Atomic' qualifier ignored; operations involving the enumeration type will be non-atomic}} + Unimportant + } e; +}; + +// Okay, was previously rejected. The underlying type is int. +S<_Atomic(int)> s; // expected-warning {{'_Atomic' is a C11 extension}} + // expected-note@-1 {{in instantiation of template class 'GH147736::S<_Atomic(int)>' requested here}} +static_assert(__is_same(__underlying_type(S<_Atomic(long long)>::OhBoy), long long), ""); // expected-warning {{'_Atomic' is a C11 extension}} + // expected-note@-1 {{in instantiation of template class 'GH147736::S<_Atomic(long long)>' requested here}} +}