Skip to content

[C23] Fix handling of alignas #81637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,19 @@ C23 Feature Support
- No longer diagnose use of binary literals as an extension in C23 mode. Fixes
`#72017 <https://github.com/llvm/llvm-project/issues/72017>`_.

- 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>`_).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should that be in the breaking change section?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can move the code example up there easily enough if you'd like.

Non-comprehensive list of changes in this release
-------------------------------------------------

Expand Down
30 changes: 22 additions & 8 deletions clang/lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preexisting but no compat warning in C11 mode?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we should probably add a compat warning here (and in a bunch of other places) -- perhaps a good first issue?

[[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;

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down
10 changes: 6 additions & 4 deletions clang/lib/Parse/ParseDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Parse/ParseTentative.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
57 changes: 39 additions & 18 deletions clang/test/C/C2x/n2934.c
Original file line number Diff line number Diff line change
@@ -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>

Expand Down Expand Up @@ -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;
};
9 changes: 1 addition & 8 deletions clang/test/Parser/c2x-alignas.c
Original file line number Diff line number Diff line change
@@ -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}}