[clang] [clang] Improve `_Alignas` on a `struct` declaration diagnostic (PR #65638)

Jerin Philip via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 1 13:43:34 PDT 2023


https://github.com/jerinphilip updated https://github.com/llvm/llvm-project/pull/65638

>From 323e569956150d46b5299129edc2db8b39c9113b Mon Sep 17 00:00:00 2001
From: Jerin Philip <jerinphilip at live.in>
Date: Sat, 19 Aug 2023 16:43:53 +0530
Subject: [PATCH] [clang] Improve _Alignas on declaration diagnostic

Adds `isAlignas()` method on `AttributeCommonInfo` which accounts for
C++ `alignas` as well as C11 `_Alignas`.

The method is used to improve diagnostic in C when `_Alignas` is used in
C at the wrong location.  This corrects the previously suggested move
of `_Alignas` past the declaration specifier, now warns attribute
`_Alignas` is ignored.
---
 clang/docs/ReleaseNotes.rst                   |  5 ++++
 .../include/clang/Basic/AttributeCommonInfo.h |  9 ++++++++
 clang/lib/Sema/SemaDecl.cpp                   | 23 +++++++++++--------
 clang/test/C/drs/dr4xx.c                      |  6 +----
 clang/test/Parser/c1x-alignas.c               |  1 +
 clang/test/Parser/c2x-alignas.c               | 11 +++++++++
 clang/test/Parser/cxx0x-attributes.cpp        |  2 ++
 7 files changed, 43 insertions(+), 14 deletions(-)
 create mode 100644 clang/test/Parser/c2x-alignas.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3198d6bfe75a2e8..5008f578a6f7375 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -288,6 +288,11 @@ Attribute Changes in Clang
   When viewing ``S::FruitKind`` in a debugger, it will behave as if the member
   was declared as type ``E`` rather than ``unsigned``.
 
+- Clang now warns you that the ``_Alignas`` attribute on declaration specifiers
+  is ignored, changed from the former incorrect suggestion to move it past
+  declaration specifiers. (`#58637: <https://github.com/llvm/llvm-project/issues/58637>`_)
+
+
 Improvements to Clang's diagnostics
 -----------------------------------
 - Clang constexpr evaluator now prints template arguments when displaying
diff --git a/clang/include/clang/Basic/AttributeCommonInfo.h b/clang/include/clang/Basic/AttributeCommonInfo.h
index 7dc05418498d0ae..a8a5d3b475d2a69 100644
--- a/clang/include/clang/Basic/AttributeCommonInfo.h
+++ b/clang/include/clang/Basic/AttributeCommonInfo.h
@@ -192,6 +192,15 @@ class AttributeCommonInfo {
 
   bool isC23Attribute() const { return SyntaxUsed == AS_C23; }
 
+  bool isAlignas() const {
+    // FIXME: In the current state, the IsAlignas member variable is only true
+    // with the C++  `alignas` keyword but not `_Alignas`. The following
+    // expression works around the otherwise lost information so it will return
+    // true for `alignas` or `_Alignas` while still returning false for things
+    // like  `__attribute__((aligned))`.
+    return (getParsedKind() == AT_Aligned && isKeywordAttribute());
+  }
+
   /// The attribute is spelled [[]] in either C or C++ mode, including standard
   /// attributes spelled with a keyword, like alignas.
   bool isStandardAttributeSyntax() const {
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index a8bad12b670fc75..57e0106e433264c 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -5343,16 +5343,21 @@ Decl *Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS,
         TypeSpecType == DeclSpec::TST_interface ||
         TypeSpecType == DeclSpec::TST_union ||
         TypeSpecType == DeclSpec::TST_enum) {
-      for (const ParsedAttr &AL : DS.getAttributes())
-        Diag(AL.getLoc(), AL.isRegularKeywordAttribute()
-                              ? diag::err_declspec_keyword_has_no_effect
-                              : diag::warn_declspec_attribute_ignored)
-            << AL << GetDiagnosticTypeSpecifierID(DS);
-      for (const ParsedAttr &AL : DeclAttrs)
-        Diag(AL.getLoc(), AL.isRegularKeywordAttribute()
-                              ? diag::err_declspec_keyword_has_no_effect
-                              : diag::warn_declspec_attribute_ignored)
+
+      auto EmitAttributeDiagnostic = [this, &DS](const ParsedAttr &AL) {
+        unsigned DiagnosticId = diag::warn_declspec_attribute_ignored;
+        if (AL.isAlignas() && !getLangOpts().CPlusPlus)
+          DiagnosticId = diag::warn_attribute_ignored;
+        else if (AL.isRegularKeywordAttribute())
+          DiagnosticId = diag::err_declspec_keyword_has_no_effect;
+        else
+          DiagnosticId = diag::warn_declspec_attribute_ignored;
+        Diag(AL.getLoc(), DiagnosticId)
             << AL << GetDiagnosticTypeSpecifierID(DS);
+      };
+
+      llvm::for_each(DS.getAttributes(), EmitAttributeDiagnostic);
+      llvm::for_each(DeclAttrs, EmitAttributeDiagnostic);
     }
   }
 
diff --git a/clang/test/C/drs/dr4xx.c b/clang/test/C/drs/dr4xx.c
index b8ccceaad12c59f..30145dcfeef1686 100644
--- a/clang/test/C/drs/dr4xx.c
+++ b/clang/test/C/drs/dr4xx.c
@@ -164,11 +164,7 @@ void dr444(void) {
   /* FIXME: This should be accepted as per this DR. */
   int j = (_Alignas(int) int){12}; /* expected-error {{expected expression}} */
 
- /* FIXME: The diagnostic in this case is really bad; moving the specifier to
-  * where the diagnostic recommends causes a different, more inscrutable error
-  * about anonymous structures.
-  */
-  _Alignas(int) struct T { /* expected-warning {{attribute '_Alignas' is ignored, place it after "struct" to apply attribute to type declaration}} */
+  _Alignas(int) struct T { /* expected-warning {{'_Alignas' attribute ignored}} */
     int i;
   };
 
diff --git a/clang/test/Parser/c1x-alignas.c b/clang/test/Parser/c1x-alignas.c
index 8f3d9bdb62d477a..bf08aabc94a8d09 100644
--- a/clang/test/Parser/c1x-alignas.c
+++ b/clang/test/Parser/c1x-alignas.c
@@ -9,5 +9,6 @@ char c4 _Alignas(32); // expected-error {{expected ';' after top level declarato
 
 char _Alignas(_Alignof(int)) c5;
 
+_Alignas(int) struct c6; // expected-warning {{'_Alignas' attribute ignored}}
 // CHECK-EXT: '_Alignas' is a C11 extension
 // CHECK-EXT: '_Alignof' is a C11 extension
diff --git a/clang/test/Parser/c2x-alignas.c b/clang/test/Parser/c2x-alignas.c
new file mode 100644
index 000000000000000..6b02b94c0a295b0
--- /dev/null
+++ b/clang/test/Parser/c2x-alignas.c
@@ -0,0 +1,11 @@
+// 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}}
diff --git a/clang/test/Parser/cxx0x-attributes.cpp b/clang/test/Parser/cxx0x-attributes.cpp
index 10c5bbcac10227b..fad3010c98b9c28 100644
--- a/clang/test/Parser/cxx0x-attributes.cpp
+++ b/clang/test/Parser/cxx0x-attributes.cpp
@@ -451,3 +451,5 @@ namespace P2361 {
                                  // expected-warning {{use of the 'deprecated' attribute is a C++14 extension}}
 [[nodiscard("\123")]] int b(); // expected-error{{invalid escape sequence '\123' in an unevaluated string literal}}
 }
+
+alignas(int) struct AlignAsAttribute {}; // expected-error {{misplaced attributes; expected attributes here}}



More information about the cfe-commits mailing list