[PATCH] D99936: [clang][parser] Unify rejecting (non) decl stmt with gnu attributes

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, rsmith.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I randomly noticed this comment (introduced in a3e01cf822f7415337e5424af3c6f4c94a12c1b9 from @rsmith),  which is now incorrect.

It suggests that decl statements and others should be handled the same here.

I'm just posting this since it seems correct (according to the comment from 2013), but I'm not sure if the new diagnostic in the test case is really better than the old one.

If the old one is preferred, I can just reword the comment instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99936

Files:
  clang/lib/Parse/ParseStmt.cpp
  clang/test/SemaCXX/warn-unused-label-error.cpp


Index: clang/test/SemaCXX/warn-unused-label-error.cpp
===================================================================
--- clang/test/SemaCXX/warn-unused-label-error.cpp
+++ clang/test/SemaCXX/warn-unused-label-error.cpp
@@ -12,8 +12,8 @@
 
   void g() {
     C: // unused label 'C' will not appear here because an error has occurred
-      __attribute__((unused))
-      #pragma weak unused_local_static  // expected-error {{expected ';' after __attribute__}}
+      __attribute__((unused)) // expected-error {{an attribute list cannot appear here}}
+      #pragma weak unused_local_static
       ;
   }
 
Index: clang/lib/Parse/ParseStmt.cpp
===================================================================
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -648,18 +648,12 @@
     // attributes as part of a statement in that case). That looks like a bug.
     if (!getLangOpts().CPlusPlus || Tok.is(tok::semi))
       attrs.takeAllFrom(TempAttrs);
-    else if (isDeclarationStatement()) {
+    else {
       StmtVector Stmts;
-      // FIXME: We should do this whether or not we have a declaration
-      // statement, but that doesn't work correctly (because ProhibitAttributes
-      // can't handle GNU attributes), so only call it in the one case where
-      // GNU attributes are allowed.
       SubStmt = ParseStatementOrDeclarationAfterAttributes(Stmts, StmtCtx,
                                                            nullptr, TempAttrs);
       if (!TempAttrs.empty() && !SubStmt.isInvalid())
         SubStmt = Actions.ActOnAttributedStmt(TempAttrs, SubStmt.get());
-    } else {
-      Diag(Tok, diag::err_expected_after) << "__attribute__" << tok::semi;
     }
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D99936.335450.patch
Type: text/x-patch
Size: 1716 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210406/660dd69c/attachment.bin>