[C23] Fix handling of alignas #81637
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In C++, alignas is an attribute specifier, while in C23, it's an alias of _Alignas, which is a type specifier/qualifier. This means that they parse differently in some circumstances.
AaronBallman
added
c23
clang:frontend
|
@llvm/pr-subscribers-clang Author: Aaron Ballman (AaronBallman) ChangesIn C++, alignas is an attribute specifier, while in C23, it's an alias of _Alignas, which is a type specifier/qualifier. This means that they parse differently in some circumstances. Fixes #81472 Full diff: https://github.com/llvm/llvm-project/pull/81637.diff 6 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dc2fb3b25e3a54..637892d143f276 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -114,6 +114,19 @@ C Language Changes
C23 Feature Support
^^^^^^^^^^^^^^^^^^^
+- Corrected parsing behavior for the ``alignas`` specifier/qualifier in C23. We
+ previously handled it as an attribute as in C++, but there are parsing
+ differences. The behavioral differences are:
+
+ .. code-block:: c
+
+ struct alignas(8) /* was accepted, now rejected */ S {
+ char alignas(8) /* was rejected, now accepted */ C;
+ };
+ int i alignas(8) /* was accepted, now rejected */ ;
+
+ Fixes (`#81472 <https://github.com/llvm/llvm-project/issues/81472>`_).
+
Non-comprehensive list of changes in this release
-------------------------------------------------
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 94696d8e482e5d..9640d7ee70d27f 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -3518,8 +3518,24 @@ void Parser::ParseDeclarationSpecifiers(
DS.Finish(Actions, Policy);
return;
- case tok::l_square:
+ // alignment-specifier
+ case tok::kw__Alignas:
+ if (!getLangOpts().C11)
+ Diag(Tok, diag::ext_c11_feature) << Tok.getName();
+ [[fallthrough]];
case tok::kw_alignas:
+ // _Alignas and alignas (C23, not C++) should parse the same way. The C++
+ // parsing for alignas happens through the usual attribute parsing. This
+ // ensures that an alignas specifier can appear in a type position in C
+ // despite that not being valid in C++.
+ if (getLangOpts().C23 || Tok.getKind() == tok::kw__Alignas) {
+ if (Tok.getKind() == tok::kw_alignas)
+ Diag(Tok, diag::warn_c23_compat_keyword) << Tok.getName();
+ ParseAlignmentSpecifier(DS.getAttributes());
+ continue;
+ }
+ [[fallthrough]];
+ case tok::l_square:
if (!isAllowedCXX11AttributeSpecifier())
goto DoneWithDeclSpec;
@@ -4234,13 +4250,6 @@ void Parser::ParseDeclarationSpecifiers(
isInvalid = DS.setFunctionSpecNoreturn(Loc, PrevSpec, DiagID);
break;
- // alignment-specifier
- case tok::kw__Alignas:
- if (!getLangOpts().C11)
- Diag(Tok, diag::ext_c11_feature) << Tok.getName();
- ParseAlignmentSpecifier(DS.getAttributes());
- continue;
-
// friend
case tok::kw_friend:
if (DSContext == DeclSpecContext::DSC_class)
@@ -5857,6 +5866,11 @@ bool Parser::isDeclarationSpecifier(
case tok::kw__Atomic:
return true;
+ case tok::kw_alignas:
+ // alignas is a type-specifier-qualifier in C23, which is a kind of
+ // declaration-specifier. Outside of C23 mode (including in C++), it is not.
+ return getLangOpts().C23;
+
// GNU ObjC bizarre protocol extension: <proto1,proto2> with implicit 'id'.
case tok::less:
return getLangOpts().ObjC;
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 79928ddb5af599..7d0dbc4ac69490 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -4661,10 +4661,12 @@ void Parser::ParseCXX11AttributeSpecifierInternal(ParsedAttributes &Attrs,
CachedTokens &OpenMPTokens,
SourceLocation *EndLoc) {
if (Tok.is(tok::kw_alignas)) {
- if (getLangOpts().C23)
- Diag(Tok, diag::warn_c23_compat_keyword) << Tok.getName();
- else
- Diag(Tok.getLocation(), diag::warn_cxx98_compat_alignas);
+ // alignas is a valid token in C23 but it is not an attribute, it's a type-
+ // specifier-qualifier, which means it has different parsing behavior. We
+ // handle this in ParseDeclarationSpecifiers() instead of here in C. We
+ // should not get here for C any longer.
+ assert(getLangOpts().CPlusPlus && "'alignas' is not an attribute in C");
+ Diag(Tok.getLocation(), diag::warn_cxx98_compat_alignas);
ParseAlignmentSpecifier(Attrs, EndLoc);
return;
}
diff --git a/clang/lib/Parse/ParseTentative.cpp b/clang/lib/Parse/ParseTentative.cpp
index f1737cb8447677..1f11f4bd937cd5 100644
--- a/clang/lib/Parse/ParseTentative.cpp
+++ b/clang/lib/Parse/ParseTentative.cpp
@@ -737,7 +737,8 @@ bool Parser::isCXXTypeId(TentativeCXXTypeIdContext Context, bool &isAmbiguous) {
Parser::CXX11AttributeKind
Parser::isCXX11AttributeSpecifier(bool Disambiguate,
bool OuterMightBeMessageSend) {
- if (Tok.is(tok::kw_alignas))
+ // alignas is an attribute specifier in C++ but not in C23.
+ if (Tok.is(tok::kw_alignas) && !getLangOpts().C23)
return CAK_AttributeSpecifier;
if (Tok.isRegularKeywordAttribute())
diff --git a/clang/test/C/C2x/n2934.c b/clang/test/C/C2x/n2934.c
index a5ee09d031914e..55de1327e3f670 100644
--- a/clang/test/C/C2x/n2934.c
+++ b/clang/test/C/C2x/n2934.c
@@ -1,26 +1,22 @@
-// RUN: %clang_cc1 -ffreestanding -verify=c2x -std=c2x -Wpre-c2x-compat %s
-// RUN: %clang_cc1 -ffreestanding -verify=c17 -std=c17 %s
+// RUN: %clang_cc1 -ffreestanding -verify=expected,c2x -std=c2x -Wpre-c2x-compat %s
+// RUN: %clang_cc1 -ffreestanding -verify=expected,c17 -std=c17 %s
/* WG14 N2934: yes
* Revise spelling of keywords v7
*/
-thread_local struct alignas(int) S { // c2x-warning {{'alignas' is incompatible with C standards before C23}} \
- c2x-warning {{'thread_local' is incompatible with C standards before C23}} \
- c2x-error 0+ {{thread-local storage is not supported for the current target}} \
- c17-error {{unknown type name 'thread_local'}} \
- c17-error {{expected identifier or '('}} \
- c17-error {{expected ')'}} \
- c17-note {{to match this '('}}
- bool b; // c2x-warning {{'bool' is incompatible with C standards before C23}}
-} s; // c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}}
-
-static_assert(alignof(struct S) == alignof(int), ""); // c2x-warning {{'static_assert' is incompatible with C standards before C23}} \
- c2x-warning 2 {{'alignof' is incompatible with C standards before C23}} \
- c17-error 2 {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}} \
- c17-error {{expected ')'}} \
- c17-warning {{declaration of 'struct S' will not be visible outside of this function}} \
- c17-note {{to match this '('}}
+thread_local struct S { // c2x-warning {{'thread_local' is incompatible with C standards before C23}} \
+ c2x-error 0+ {{thread-local storage is not supported for the current target}} \
+ c17-error {{unknown type name 'thread_local'}}
+ bool b; // c2x-warning {{'bool' is incompatible with C standards before C23}} \
+ c17-error {{unknown type name 'bool'}}
+} s;
+
+static_assert(alignof(int) != 0, ""); // c2x-warning {{'static_assert' is incompatible with C standards before C23}} \
+ c2x-warning {{'alignof' is incompatible with C standards before C23}} \
+ c17-error 2 {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}} \
+ c17-error {{expected ')'}} \
+ c17-note {{to match this '('}}
#include <stdalign.h>
@@ -56,3 +52,28 @@ static_assert(alignof(struct S) == alignof(int), ""); // c2x-warning {{'static_a
#error "bool should not be defined"
#endif
#endif
+
+// Ensure we correctly parse the alignas keyword in a specifier-qualifier-list.
+// This is different than in C++ where alignas is an actual attribute rather
+// than a specifier.
+struct GH81472 {
+ char alignas(8) a1; // c2x-warning {{'alignas' is incompatible with C standards before C23}}
+ alignas(8) char a2; // c2x-warning {{'alignas' is incompatible with C standards before C23}}
+ char _Alignas(8) a3;
+ _Alignas(8) char a4;
+ char a5 alignas(8); // expected-error {{expected ';' at end of declaration list}}
+ char a6 _Alignas(8); // expected-error {{expected ';' at end of declaration list}}
+};
+
+// Ensure we reject alignas as an attribute specifier. This code is accepted in
+// C++ mode but should be rejected in C.
+// FIXME: this diagnostic could be improved
+struct alignas(8) Reject1 { // expected-error {{declaration of anonymous struct must be a definition}} \
+ expected-warning {{declaration does not declare anything}}
+ int a;
+};
+
+struct _Alignas(8) Reject2 { // expected-error {{declaration of anonymous struct must be a definition}} \
+ expected-warning {{declaration does not declare anything}}
+ int a;
+};
diff --git a/clang/test/Parser/c2x-alignas.c b/clang/test/Parser/c2x-alignas.c
index 6b02b94c0a295b..1658cb1c744966 100644
--- a/clang/test/Parser/c2x-alignas.c
+++ b/clang/test/Parser/c2x-alignas.c
@@ -1,11 +1,4 @@
// RUN: %clang_cc1 -std=c23 -fsyntax-only -verify %s
_Alignas(int) struct c1; // expected-warning {{'_Alignas' attribute ignored}}
-
-// FIXME: `alignas` enters into C++ parsing code and never reaches the
-// declaration specifier attribute diagnostic infrastructure.
-//
-// Fixing this will require the C23 notions of `alignas` being a keyword and
-// `_Alignas` being an alternate spelling integrated into the parsing
-// infrastructure.
-alignas(int) struct c1; // expected-error {{misplaced attributes; expected attributes here}}
+alignas(int) struct c1; // expected-warning {{'alignas' attribute ignored}}
|
cor3ntin
approved these changes
Feb 13, 2024
| int i alignas(8) /* was accepted, now rejected */ ; | ||
|
|
||
| Fixes (`#81472 <https://github.com/llvm/llvm-project/issues/81472>`_). | ||
|
|
There was a problem hiding this comment.
Should that be in the breaking change section?
There was a problem hiding this comment.
I can move the code example up there easily enough if you'd like.
| // alignment-specifier | ||
| case tok::kw__Alignas: | ||
| if (!getLangOpts().C11) | ||
| Diag(Tok, diag::ext_c11_feature) << Tok.getName(); |
There was a problem hiding this comment.
Preexisting but no compat warning in C11 mode?
There was a problem hiding this comment.
Yeah, we should probably add a compat warning here (and in a bunch of other places) -- perhaps a good first issue?
cor3ntin
approved these changes
Feb 15, 2024
Yes, or I'll fix the issue myself. One or the other. :-) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In C++, alignas is an attribute specifier, while in C23, it's an alias of _Alignas, which is a type specifier/qualifier. This means that they parse differently in some circumstances.
Fixes #81472