[clang] [C23] Fix handling of alignas (PR #81637)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 13 09:51:16 PST 2024


https://github.com/AaronBallman created https://github.com/llvm/llvm-project/pull/81637

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.

>From 5df680f0f335e399ddac73421777ebeccc50c968 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Tue, 13 Feb 2024 12:49:03 -0500
Subject: [PATCH] [C23] Fix handling of alignas

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.
---
 clang/docs/ReleaseNotes.rst        | 13 +++++++
 clang/lib/Parse/ParseDecl.cpp      | 30 +++++++++++-----
 clang/lib/Parse/ParseDeclCXX.cpp   | 10 +++---
 clang/lib/Parse/ParseTentative.cpp |  3 +-
 clang/test/C/C2x/n2934.c           | 57 ++++++++++++++++++++----------
 clang/test/Parser/c2x-alignas.c    |  9 +----
 6 files changed, 83 insertions(+), 39 deletions(-)

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}}



More information about the cfe-commits mailing list