[C23] Accept an _Atomic underlying type #147802

AaronBallman · 2025-07-09T18:42:32Z

The underlying type of an enumeration is the non-atomic, unqualified version of the specified type. Clang was rejecting such enumerations, but now accepts them.

Because we expose _Atomic in C++, the same behavior is carried over.

Fixes #147736

The underlying type of an enumeration is the non-atomic, unqualified version of the specified type. Clang was rejecting such enumerations, but now accepts them. Because we expose _Atomic in C++, the same behavior is carried over. Fixes llvm#147736

llvmbot · 2025-07-09T18:42:54Z

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

The underlying type of an enumeration is the non-atomic, unqualified version of the specified type. Clang was rejecting such enumerations, but now accepts them.

Because we expose _Atomic in C++, the same behavior is carried over.

Fixes #147736

Full diff: https://github.com/llvm/llvm-project/pull/147802.diff

5 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+3)
(modified) clang/lib/Sema/SemaDecl.cpp (+20)
(modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+8-2)
(added) clang/test/C/C23/n3030_1.c (+13)
(modified) clang/test/SemaCXX/enum-scoped.cpp (+13)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 57a94242c9e61..4fc74f430e768 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -310,6 +310,9 @@ C23 Feature Support
   `WG14 N2975 <https://open-std.org/JTC1/SC22/WG14/www/docs/n2975.pdf>`_
 - 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. (#GH147736)
 
 C11 Feature Support
 ^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 11cbda412667f..e2c99e1bb0cb2 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -17152,6 +17152,13 @@ bool Sema::CheckEnumUnderlyingType(TypeSourceInfo *TI) {
   SourceLocation UnderlyingLoc = TI->getTypeLoc().getBeginLoc();
   QualType T = TI->getType();
 
+  // 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.
+  T = T.getAtomicUnqualifiedType();
+
   if (T->isDependentType())
     return false;
 
@@ -17551,6 +17558,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 +17573,16 @@ 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.
+      if (TypeSourceInfo *TI = dyn_cast<TypeSourceInfo *>(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..1608232f4553e 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2022,8 +2022,14 @@ 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().
+        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_1.c b/clang/test/C/C23/n3030_1.c
new file mode 100644
index 0000000000000..4acfc007709ac
--- /dev/null
+++ b/clang/test/C/C23/n3030_1.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c23 -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/SemaCXX/enum-scoped.cpp b/clang/test/SemaCXX/enum-scoped.cpp
index d7b7923430aff..f287d47107bc9 100644
--- a/clang/test/SemaCXX/enum-scoped.cpp
+++ b/clang/test/SemaCXX/enum-scoped.cpp
@@ -349,3 +349,16 @@ enum class B;
 A a;
 B b{a}; // expected-error {{cannot initialize}}
 }
+
+namespace GH147736 {
+template <typename Ty>
+struct S {
+  enum OhBoy : Ty {
+    Unimportant
+  } e;
+};
+
+// Okay, was previously rejected. The underlying type is int.
+S<_Atomic(int)> s; // expected-warning {{'_Atomic' is a C11 extension}}
+static_assert(__is_same(__underlying_type(S<_Atomic(long long)>::OhBoy), long long), ""); // expected-warning {{'_Atomic' is a C11 extension}}
+}

erichkeane

I THINK this is right, but we also need to have a codegen test to make sure we don't try to do goofiness with these.

erichkeane · 2025-07-09T18:46:41Z

clang/lib/Sema/SemaDecl.cpp

+  // 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.
+  T = T.getAtomicUnqualifiedType();


Since we're now stripping qualifications, we should probably make sure that all of the callers of this 'treat' that like underlying.

clang/lib/Sema/SemaDecl.cpp

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

clang/lib/Sema/SemaDecl.cpp

AaronBallman · 2025-07-09T19:00:16Z

I THINK this is right, but we also need to have a codegen test to make sure we don't try to do goofiness with these.

I was thinking the ast-dump test sufficed because it shows that we set the underlying type to non-atomic in the AST. But I can certainly add a codegen test, do you have anything in particular in mind?

erichkeane · 2025-07-09T19:03:42Z

I THINK this is right, but we also need to have a codegen test to make sure we don't try to do goofiness with these.

I was thinking the ast-dump test sufficed because it shows that we set the underlying type to non-atomic in the AST. But I can certainly add a codegen test, do you have anything in particular in mind?

Ah, yeah, that works. I missed that was validating CV-A refs didn't make it into the AST. Concern abaited :)

But the test I was looking for was basically just a load/store to a variable of that type to make sure we treated it like an 'int' type not an _Atomic int type. So that a store would look like:

  store i32 5, ptr %i, align 4

instead of :

  store atomic i32 6, ptr %AI seq_cst, align 4

AaronBallman · 2025-07-09T19:20:13Z

I went ahead and added a codegen test, better to test than hope. :-)

AaronBallman · 2025-07-10T11:30:54Z

I thought about this more overnight and I think I want to make a change here -- a user who sees enum E : _Atomic(int) may be reasonably thinking that values of type enum E are going to be atomically accessed, same as when they see typedef _Atomic(int) atomic_int. But they're not going to get the atomic behavior due to the silent stripping. Given that an atomic type is not the same as its underlying type (in terms of ABI or semantics) I think we should diagnose the behavior with at least a warning. I could even be convinced it should be a warning which defaults to an error because this is just weird.

Consider:

enum E : _Atomic(int) { Whatever };
static_assert(sizeof(enum E) == sizeof(_Atomic(int))); // Surprise! This can fail

because an atomic type is not required to have the same size as its non-atomic counterpart, per 7.17.6.p3.

erichkeane · 2025-07-10T13:27:26Z

avior due to the silent stripping. Given that an atomic type is not the same as its underlying type (in terms of ABI or semantics) I think we should diagnose the behavior with at least a warning. I could even be convinced it should be a warning which defaults to an error because this is just weird.

Yeah, I 100% agree with that. Warn-as-error is reasonable here IMO (as are ANY qualifiers?, but more so for atomic)

erichkeane

Would like a pair of fixmes, but this is otherwise good to me.

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

AaronBallman · 2025-07-10T13:45:51Z

avior due to the silent stripping. Given that an atomic type is not the same as its underlying type (in terms of ABI or semantics) I think we should diagnose the behavior with at least a warning. I could even be convinced it should be a warning which defaults to an error because this is just weird.

Yeah, I 100% agree with that. Warn-as-error is reasonable here IMO (as are ANY qualifiers?, but more so for atomic)

Would you like me to diagnose any qualifier? And turn it into warning-as-error?

AaronBallman requested review from erichkeane, Fznamznon, efriedma-quic and Sirraide July 9, 2025 18:42

AaronBallman added clang Clang issues not falling into any other category c23 clang:frontend Language frontend issues, e.g. anything involving "Sema" rejects-valid labels Jul 9, 2025

erichkeane reviewed Jul 9, 2025

View reviewed changes

cor3ntin reviewed Jul 9, 2025

View reviewed changes

clang/lib/Sema/SemaDecl.cpp Outdated Show resolved Hide resolved

Move conversion to after the check for type dependence

8f5ce34

Add a codegen test

1df979f

Add a diagnostic

9ba10c7

erichkeane approved these changes Jul 10, 2025

View reviewed changes

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp Show resolved Hide resolved

Add some FIXMEs

45517ca

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[C23] Accept an _Atomic underlying type #147802

[C23] Accept an _Atomic underlying type #147802

AaronBallman commented Jul 9, 2025

Uh oh!

llvmbot commented Jul 9, 2025

Uh oh!

erichkeane left a comment

Uh oh!

erichkeane Jul 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AaronBallman commented Jul 9, 2025

Uh oh!

erichkeane commented Jul 9, 2025

Uh oh!

AaronBallman commented Jul 9, 2025

Uh oh!

AaronBallman commented Jul 10, 2025

Uh oh!

erichkeane commented Jul 10, 2025

Uh oh!

erichkeane left a comment

Uh oh!

Uh oh!

AaronBallman commented Jul 10, 2025

Uh oh!

Uh oh!

[C23] Accept an _Atomic underlying type #147802

Are you sure you want to change the base?

[C23] Accept an _Atomic underlying type #147802

Conversation

AaronBallman commented Jul 9, 2025

Uh oh!

llvmbot commented Jul 9, 2025

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

erichkeane Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AaronBallman commented Jul 9, 2025

Uh oh!

erichkeane commented Jul 9, 2025

Uh oh!

AaronBallman commented Jul 9, 2025

Uh oh!

AaronBallman commented Jul 10, 2025

Uh oh!

erichkeane commented Jul 10, 2025

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AaronBallman commented Jul 10, 2025

Uh oh!

Uh oh!