[PATCH] D141177: [Clang] Don't tell people to place _Alignas on a struct in diagnostics

[Theodore Luo Wang via Phabricator via cfe-commits]

theo-lw updated this revision to Diff 552061.
theo-lw added a comment.
Herald added a subscriber: jdoerfert.

Rebased on the main branch & addressed comments.

Added additional test case for C++ alignas.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141177/new/

https://reviews.llvm.org/D141177

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/AttributeCommonInfo.h
  clang/lib/Sema/SemaDecl.cpp
  clang/test/C/drs/dr4xx.c
  clang/test/Parser/c1x-alignas.c
  clang/test/Parser/cxx0x-attributes.cpp


Index: clang/test/Parser/cxx0x-attributes.cpp
===================================================================
--- clang/test/Parser/cxx0x-attributes.cpp
+++ clang/test/Parser/cxx0x-attributes.cpp
@@ -451,3 +451,5 @@
                                  // 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}}
Index: clang/test/Parser/c1x-alignas.c
===================================================================
--- clang/test/Parser/c1x-alignas.c
+++ clang/test/Parser/c1x-alignas.c
@@ -9,5 +9,7 @@
 
 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
Index: clang/test/C/drs/dr4xx.c
===================================================================
--- clang/test/C/drs/dr4xx.c
+++ clang/test/C/drs/dr4xx.c
@@ -164,11 +164,7 @@
   /* 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;
   };
 
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5333,22 +5333,28 @@
   // __attribute__((aligned)) struct A;
   // Attributes should be placed after tag to apply to type declaration.
   if (!DS.getAttributes().empty() || !DeclAttrs.empty()) {
+    auto WarnAttributeIgnored = [this, &DS](const ParsedAttr &AL) {
+      if (AL.isC11AlignasAttribute()) {
+        // Don't use the message with placement with _Alignas.
+        // This is because C doesnt let you use _Alignas on type declarations.
+        Diag(AL.getLoc(), diag::warn_attribute_ignored)
+            << AL;
+      } else {
+        Diag(AL.getLoc(), AL.isRegularKeywordAttribute()
+                              ? diag::err_declspec_keyword_has_no_effect
+                              : diag::warn_declspec_attribute_ignored)
+            << AL << GetDiagnosticTypeSpecifierID(DS);
+      }
+    };
+
     DeclSpec::TST TypeSpecType = DS.getTypeSpecType();
     if (TypeSpecType == DeclSpec::TST_class ||
         TypeSpecType == DeclSpec::TST_struct ||
         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)
-            << AL << GetDiagnosticTypeSpecifierID(DS);
+      llvm::for_each(DS.getAttributes(), WarnAttributeIgnored);
+      llvm::for_each(DeclAttrs, WarnAttributeIgnored);
     }
   }
 
Index: clang/include/clang/Basic/AttributeCommonInfo.h
===================================================================
--- clang/include/clang/Basic/AttributeCommonInfo.h
+++ clang/include/clang/Basic/AttributeCommonInfo.h
@@ -186,6 +186,10 @@
   bool isGNUScope() const;
   bool isClangScope() const;
 
+  bool isC11AlignasAttribute() const {
+    return getParsedKind() == AT_Aligned && isKeywordAttribute() && !IsAlignas;
+  }
+
   bool isCXX11Attribute() const { return SyntaxUsed == AS_CXX11 || IsAlignas; }
 
   bool isC23Attribute() const { return SyntaxUsed == AS_C23; }
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -142,6 +142,8 @@
   tautologies like ``x && !x`` and ``!x || x`` in expressions. This also
   makes ``-Winfinite-recursion`` diagnose more cases.
   (`#56035: <https://github.com/llvm/llvm-project/issues/56035>`_).
+- Clang now informs you that the ``_Alignas`` attribute on a struct is ignored, instead of
+  suggesting that it should be moved.
 
 Bug Fixes in This Version
 -------------------------

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits