[clang][sema] Add support and documentation for `__has_extension(c_fixed_enum)` #117507

AidanGoldfarb · 2024-11-24T20:40:10Z

This PR addresses #116880

Updated LanguageExtensions.rst to include support for C++11 enumerations with a fixed underlying type in C. Included a note that this is only a language extension prior to C23.

Updated Features.def to support for __has_extension(c_fixed_enum) by added it as a feature (for C23) and an extension (for <C23).

Updated enum.c to ensure support of C++11 enumerations with a fixed underlying type in both <C23 and C23, as well as the functionality of __has_extension(c_fixed_enum).

In enum.c, I encountered a warning when testing enumerations with a fixed underlying type in pre-C23 modes. Specifically, the test produces the warning: enumeration types with a fixed underlying type are a C23 extension. I am unsure if this warning is expected behavior, as enumerations with a fixed underlying type should behave identically in pre-C23 and C23 modes. I expected that adding c_fixed_enum as an extension would fix this warning. Feedback on whether this is correct or requires adjustment would be appreciated.

I was also unsure of the best location for the additions in Features.def, I would appreciate advice on this as well

Note that this is my first PR to LLVM, so please liberally critique it!

…check for support of enums with a fixed underlying type in C23 and <C23.

github-actions · 2024-11-24T20:40:30Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

justinfargnoli

Add a title.

And welcome to the LLVM community :)

clang/test/Sema/enum.c

llvmbot · 2024-11-24T21:45:10Z

@llvm/pr-subscribers-clang

Author: Aidan Goldfarb (AidanGoldfarb)

Changes

This PR addresses #116880

Updated LanguageExtensions.rst to include support for C++11 enumerations with a fixed underlying type in C. Included a note that this is only a language extension prior to C23.

Updated Features.def to support for __has_extension(c_fixed_enum) by added it as a feature (for C23) and an extension (for <C23).

Updated enum.c to ensure support of C++11 enumerations with a fixed underlying type in both <C23 and C23, as well as the functionality of __has_extension(c_fixed_enum).

In enum.c, I encountered a warning when testing enumerations with a fixed underlying type in pre-C23 modes. Specifically, the test produces the warning: enumeration types with a fixed underlying type are a C23 extension. I am unsure if this warning is expected behavior, as enumerations with a fixed underlying type should behave identically in pre-C23 and C23 modes. I expected that adding c_fixed_enum as an extension would fix this warning. Feedback on whether this is correct or requires adjustment would be appreciated.

Note that this is my first PR to LLVM, so please liberally critique it!

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

3 Files Affected:

(modified) clang/docs/LanguageExtensions.rst (+4-1)
(modified) clang/include/clang/Basic/Features.def (+4)
(modified) clang/test/Sema/enum.c (+9)

diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index 3c9078bcdf8118..1c400d87c4948b 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -1986,7 +1986,7 @@ Enumerations with a fixed underlying type
 -----------------------------------------
 
 Clang provides support for C++11 enumerations with a fixed underlying type
-within Objective-C.  For example, one can write an enumeration type as:
+within Objective-C and C `prior to C23 <https://open-std.org/JTC1/SC22/WG14/www/docs/n3030.htm>`_.  For example, one can write an enumeration type as:
 
 .. code-block:: c++
 
@@ -1998,6 +1998,9 @@ value, is ``unsigned char``.
 Use ``__has_feature(objc_fixed_enum)`` to determine whether support for fixed
 underlying types is available in Objective-C.
 
+Use ``__has_extension(c_fixed_enum)`` to determine whether support for fixed
+underlying types is available in C.
+
 Interoperability with C++11 lambdas
 -----------------------------------
 
diff --git a/clang/include/clang/Basic/Features.def b/clang/include/clang/Basic/Features.def
index 9088c867d53ce4..ab963a876db342 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -308,6 +308,10 @@ EXTENSION(datasizeof, LangOpts.CPlusPlus)
 
 FEATURE(cxx_abi_relative_vtable, LangOpts.CPlusPlus && LangOpts.RelativeCXXABIVTables)
 
+//Fixed enum feature and extension, to be relocated in this file
+FEATURE(c_fixed_enum, true)    
+EXTENSION(c_fixed_enum, true)  
+
 // CUDA/HIP Features
 FEATURE(cuda_noinline_keyword, LangOpts.CUDA)
 EXTENSION(cuda_implicit_host_device_templates, LangOpts.CUDA && LangOpts.OffloadImplicitHostDeviceTemplates)
diff --git a/clang/test/Sema/enum.c b/clang/test/Sema/enum.c
index 4f6d04ba7f9182..7b3f7d30e91d82 100644
--- a/clang/test/Sema/enum.c
+++ b/clang/test/Sema/enum.c
@@ -121,6 +121,15 @@ int NegativeShortTest[NegativeShort == -1 ? 1 : -1];
 enum Color { Red, Green, Blue }; // expected-note{{previous use is here}}
 typedef struct Color NewColor; // expected-error {{use of 'Color' with tag type that does not match previous declaration}}
 
+// Enumerations with a fixed underlying type. 
+// https://github.com/llvm/llvm-project/issues/116880
+#if __STDC_VERSION__ >= 202311L
+    typedef enum : unsigned char { Pink, Black, Cyan } Color;
+#else
+    _Static_assert(__has_extension(c_fixed_enum), "Ensure language extension support for enumerations with a fixed underlying type in <C23");
+    typedef enum : unsigned char { Pink, Black, Cyan } Color; // expected-warning {{enumeration types with a fixed underlying type are a C23 extension}}
+#endif
+
 // PR28903
 // In C it is valid to define tags inside enums.
 struct PR28903 {

AidanGoldfarb · 2024-11-25T04:32:29Z

Ping @pinskia @AaronBallman for review.

pinskia · 2024-11-25T04:36:46Z

From my point of view this looks like a decent documentation of this extension. Note I filed the original bug because I noticed both GCC and clang didn't document this extension. (I filed GCC here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117689 which I got try fix next week).

Sirraide · 2024-11-25T12:03:11Z

clang/include/clang/Basic/Features.def

+//Fixed enum feature and extension, to be relocated in this file
+FEATURE(c_fixed_enum, true)    
+EXTENSION(c_fixed_enum, true)  


Haven’t looked at the features code in a bit, but does this need to be both a FEATURE and an EXTENSION?

CC @AaronBallman

Yeah, it should be both, but I'd move the code around a bit, similar to:

llvm-project/clang/include/clang/Basic/Features.def

Line 160 in 4ab298b

FEATURE(c_alignas, LangOpts.C11)

(we can add a new section for C23 features)

llvm-project/clang/include/clang/Basic/Features.def

Line 263 in 4ab298b

EXTENSION(c_alignas, true)

(we can put under the C23 section)

clang/test/Sema/enum.c

AaronBallman

Thank you for the work on this! It's pretty close to ready, IMO.

AaronBallman · 2024-11-26T18:38:55Z

clang/include/clang/Basic/Features.def

+//Fixed enum feature and extension, to be relocated in this file
+FEATURE(c_fixed_enum, true)    
+EXTENSION(c_fixed_enum, true)  


Yeah, it should be both, but I'd move the code around a bit, similar to:

llvm-project/clang/include/clang/Basic/Features.def

Line 160 in 4ab298b

FEATURE(c_alignas, LangOpts.C11)

(we can add a new section for C23 features)

llvm-project/clang/include/clang/Basic/Features.def

Line 263 in 4ab298b

EXTENSION(c_alignas, true)

(we can put under the C23 section)

AaronBallman · 2024-11-26T18:39:21Z

clang/test/Sema/enum.c

@@ -121,6 +121,14 @@ int NegativeShortTest[NegativeShort == -1 ? 1 : -1];
 enum Color { Red, Green, Blue }; // expected-note{{previous use is here}}
 typedef struct Color NewColor; // expected-error {{use of 'Color' with tag type that does not match previous declaration}}

+// Enumerations with a fixed underlying type. 
+// https://github.com/llvm/llvm-project/issues/116880
+#if __STDC_VERSION__ >= 202311L && !__has_extension(c_fixed_enum)


We should also add a similar test for __has_feature.

Updated this, my thought process is to check for __has_extension() when <C23, and __has_feature in >=C23.
Previously I had only checked for __has_extension() when >=C23, which in my mind was both incomplete (__has_feature() missing) and incorrect (__has_extension() should be associated with <C23).

…tures.def`.

justinfargnoli · 2024-11-27T16:35:42Z

clang/include/clang/Basic/Features.def

@@ -156,13 +156,15 @@ FEATURE(objc_class_property, LangOpts.ObjC)
 FEATURE(objc_c_static_assert, LangOpts.C11)
 FEATURE(objc_cxx_static_assert, LangOpts.CPlusPlus11)
 EXTENSION(objc_c_static_assert, true)
-// C11 features
+// C11 features 


Delete space

AaronBallman · 2024-11-27T18:26:42Z

clang/docs/LanguageExtensions.rst

+Use ``__has_extension(c_fixed_enum)`` to determine whether support for fixed
+underlying types is available in C.


We should probably update the wording here to mention __has_feature and language version behavior.

AaronBallman · 2024-11-27T18:27:16Z

clang/include/clang/Basic/Features.def

@@ -163,6 +163,8 @@ FEATURE(c_atomic, LangOpts.C11)
 FEATURE(c_generic_selections, LangOpts.C11)
 FEATURE(c_static_assert, LangOpts.C11)
 FEATURE(c_thread_local, LangOpts.C11 &&PP.getTargetInfo().isTLSSupported())
+// C23 features
+FEATURE(c_fixed_enum, true) 


Suggested change

FEATURE(c_fixed_enum, true)

FEATURE(c_fixed_enum, LangOpts.C23)

This is only a feature of C23, it's an extension in other modes.

AaronBallman · 2024-11-27T18:30:33Z

clang/test/Sema/enum.c

+#if __STDC_VERSION__ >= 202311L && !__has_feature(c_fixed_enum)
+    #error c_fixed_enum should be set a feature in C23 mode
+#elif __STDC_VERSION__ < 202311L && !__has_extension(c_fixed_enum)
+    #error c_fixed_enum should be a language extension in <C23 mode
+#else
+    typedef enum : unsigned char { Pink, Black, Cyan } Color; // pre-c23-warning {{enumeration types with a fixed underlying type are a C23 extension}}
+#endif


How about something along the lines of:

#if __STDC_VERSION__ >= 202311L static_assert(__has_feature(c_fixed_enum)); static_assert(__has_extension(c_fixed_enum)); // Matches behavior for c_alignas, etc #else _Static_assert(__has_extension(c_fixed_enum), ""); _Static_assert(!__has_feature(c_fixed_enum), ""); #endif

Updated. I chose to include
typedef enum : unsigned char { Pink, Black, Cyan } Color; // pre-c23-warning {{enumeration types with a fixed underlying type are a C23 extension}}
in enum.c, as I think we want to test for both the behavior of __has_feature() and __has_extension() as well as the actual definition of a typed enum. Please let me know if this is not a good idea!

…ne `c_fixed_enum` as a C23 feature only. Updated LanguageExtension.rst to reflect `c_fixed_enum` as a feature and extension depending on C version

AaronBallman

LGTM aside from some suggested edits to the documentation. Mostly just adding full stops to the end of sentences, and a mention about the behavior of __has_extension(c_fixed_enum) in C23 and later.

clang/docs/LanguageExtensions.rst

Co-authored-by: Aaron Ballman <[email protected]>

github-actions · 2024-12-03T18:39:39Z

@AidanGoldfarb Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

llvm-ci · 2024-12-03T18:52:00Z

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-sie-ubuntu-fast running on sie-linux-worker while building clang at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/13023

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'Clang :: Sema/enum.c' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 1: /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/bin/clang -cc1 -internal-isystem /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/lib/clang/20/include -nostdsysteminc -triple x86_64-scei-ps4 /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/Sema/enum.c -fsyntax-only -verify=expected,pre-c23 -pedantic
+ /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/bin/clang -cc1 -internal-isystem /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/lib/clang/20/include -nostdsysteminc -triple x86_64-scei-ps4 /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/Sema/enum.c -fsyntax-only -verify=expected,pre-c23 -pedantic
error: 'expected-warning' diagnostics seen but not expected: 
  File /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/Sema/enum.c Line 130: '_Static_assert' is a C11 extension
  File /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/clang/test/Sema/enum.c Line 131: '_Static_assert' is a C11 extension
2 errors generated.

--

********************

AaronBallman · 2024-12-03T18:59:08Z

That issue should be resolved by 25b1896

AidanGoldfarb and others added 2 commits November 22, 2024 19:06

Update LanguageExtensions.rst

a6c9b5c

Updated Features.def to include c_fixed_enum. Updated enum.c test to …

d9e0615

…check for support of enums with a fixed underlying type in C23 and <C23.

Added c_fixed_enum as an extension. Cleaned up enum.c

9fd4a25

justinfargnoli reviewed Nov 24, 2024

View reviewed changes

clang/test/Sema/enum.c Outdated Show resolved Hide resolved

AidanGoldfarb changed the title ~~[clang][sema]~~ [clang][sema] Add support and documentation for __has_extension(c_fixed_enum) Nov 24, 2024

formatting changes

53ba18d

AidanGoldfarb mentioned this pull request Nov 24, 2024

Request Commit Access For AidanGoldfarb #117508

Closed

AidanGoldfarb marked this pull request as ready for review November 24, 2024 21:44

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 24, 2024

justinfargnoli requested a review from AaronBallman November 25, 2024 06:17

Sirraide reviewed Nov 25, 2024

View reviewed changes

fixed enum.c typed enum logic

8722c53

AidanGoldfarb force-pushed the main branch from 572f9db to 8722c53 Compare November 25, 2024 16:16

reverted to using pre-c23-warning

3a10186

AaronBallman reviewed Nov 26, 2024

View reviewed changes

Added __has_feature(c_fixed_enum) test to enum.c. Reoganized `Fea…

7925096

…tures.def`.

justinfargnoli reviewed Nov 27, 2024

View reviewed changes

Remove extra space in Features.def

e06f739

justinfargnoli assigned AidanGoldfarb Nov 27, 2024

AaronBallman reviewed Nov 27, 2024

View reviewed changes

Updated enum.c to use static assertions. Updated Features.def to defi…

4a467b9

…ne `c_fixed_enum` as a C23 feature only. Updated LanguageExtension.rst to reflect `c_fixed_enum` as a feature and extension depending on C version

justinfargnoli requested a review from AaronBallman December 2, 2024 17:53

AaronBallman approved these changes Dec 3, 2024

View reviewed changes

clang/docs/LanguageExtensions.rst Outdated Show resolved Hide resolved

Update clang/docs/LanguageExtensions.rst

728a2d9

Co-authored-by: Aaron Ballman <[email protected]>

AaronBallman merged commit 9791f25 into llvm:main Dec 3, 2024
4 of 6 checks passed

erichkeane mentioned this pull request Jan 16, 2025

Enumerations with a fixed underlying type documentation issue #116880

Closed

		Use ``__has_extension(c_fixed_enum)`` to determine whether support for fixed
		underlying types is available in C.

	FEATURE(c_fixed_enum, true)
	FEATURE(c_fixed_enum, LangOpts.C23)

[clang][sema] Add support and documentation for __has_extension(c_fixed_enum) #117507

[clang][sema] Add support and documentation for __has_extension(c_fixed_enum) #117507

Uh oh!

Conversation

AidanGoldfarb commented Nov 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 24, 2024

Uh oh!

justinfargnoli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvmbot commented Nov 24, 2024

Uh oh!

AidanGoldfarb commented Nov 25, 2024 • edited by justinfargnoli Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pinskia commented Nov 25, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AidanGoldfarb Nov 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 3, 2024

Uh oh!

llvm-ci commented Dec 3, 2024

Uh oh!

AaronBallman commented Dec 3, 2024

Uh oh!

Uh oh!

[clang][sema] Add support and documentation for `__has_extension(c_fixed_enum)` #117507

[clang][sema] Add support and documentation for `__has_extension(c_fixed_enum)` #117507

AidanGoldfarb commented Nov 24, 2024 •

edited

Loading

AidanGoldfarb commented Nov 25, 2024 •

edited by justinfargnoli

Loading

AidanGoldfarb Nov 27, 2024 •

edited

Loading