[clang-format] BreakAfterAttributes did not take into account gnu attributes

[mydeveloperday]

Not completely sure if we should handle this with a separate option or just treat [[attribute]] the same as __attribute((attribute))

This was raised by #77310

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: MyDeveloperDay (mydeveloperday)

Changes

Not completely sure if we should handle this with a separate option or just treat [[attribute]] the same as __attribute((attribute))

This was raised by #77310

---

Full diff: https://github.com/llvm/llvm-project/pull/78102.diff

3 Files Affected:

• (modified) clang/lib/Format/TokenAnnotator.cpp (+5)
• (modified) clang/unittests/Format/FormatTest.cpp (+25)
• (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+18)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 24ce18a64348c1..ffe8428005b41d 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -489,6 +489,10 @@ class AnnotatingParser {
 
         if (OpeningParen.is(TT_AttributeLParen))
           CurrentToken->setType(TT_AttributeRParen);
+        if (CurrentToken->is(TT_AttributeRParen) && CurrentToken->Next &&
+            !CurrentToken->Next->is(tok::kw___attribute)) {
+          CurrentToken->EndsCppAttributeGroup = true;
+        }
         if (OpeningParen.is(TT_TypeDeclarationParen))
           CurrentToken->setType(TT_TypeDeclarationParen);
         if (OpeningParen.Previous &&
@@ -983,6 +987,7 @@ class AnnotatingParser {
         CurrentToken->MustBreakBefore = true;
       }
     }
+
     FormatToken *Tok = CurrentToken;
     next();
     // In Verilog primitives' state tables, `:`, `?`, and `-` aren't normal
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 8f115fb8cbf0fb..2d85395d3c540f 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -26827,6 +26827,31 @@ TEST_F(FormatTest, BreakAdjacentStringLiterals) {
   Style.BreakAdjacentStringLiterals = false;
   verifyFormat(Code, Style);
 }
+
+TEST_F(FormatTest, BreakAfterGNUAttributes) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakAfterAttributes = FormatStyle::ABS_Never;
+
+  verifyFormat("__attribute__((__warn_unused_result__)) const int i;", Style);
+
+  verifyFormat(
+      "__attribute__((other)) __attribute__((__warn_unused_result__)) int j;",
+      Style);
+
+  verifyFormat("__attribute__((__warn_unused_result__)) inline int f(int &i);",
+               Style);
+
+  Style.BreakAfterAttributes = FormatStyle::ABS_Always;
+  verifyFormat("__attribute__((__warn_unused_result__))\nconst int i;", Style);
+
+  verifyFormat(
+      "__attribute__((other)) __attribute__((__warn_unused_result__))\nint j;",
+      Style);
+
+  verifyFormat("__attribute__((__warn_unused_result__))\ninline int f(int &i);",
+               Style);
+}
+
 } // namespace
 } // namespace test
 } // namespace format
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 92f57a77cdaf01..2b255e89307f23 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2381,11 +2381,14 @@ TEST_F(TokenAnnotatorTest, UnderstandsAttributes) {
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_Unknown);
   EXPECT_TOKEN(Tokens[6], tok::r_paren, TT_Unknown);
   EXPECT_TOKEN(Tokens[7], tok::r_paren, TT_AttributeRParen);
+  EXPECT_FALSE(Tokens[6]->EndsCppAttributeGroup);
+  EXPECT_TRUE(Tokens[7]->EndsCppAttributeGroup);
 
   Tokens = annotate("bool foo __declspec(dllimport);");
   ASSERT_EQ(Tokens.size(), 8u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_AttributeLParen);
   EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_AttributeRParen);
+  EXPECT_TRUE(Tokens[5]->EndsCppAttributeGroup);
 
   Tokens = annotate("bool __attribute__((unused)) foo;");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
@@ -2394,6 +2397,21 @@ TEST_F(TokenAnnotatorTest, UnderstandsAttributes) {
   EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_Unknown);
   EXPECT_TOKEN(Tokens[6], tok::r_paren, TT_AttributeRParen);
   EXPECT_TOKEN(Tokens[7], tok::identifier, TT_StartOfName);
+  EXPECT_TRUE(Tokens[6]->EndsCppAttributeGroup);
+
+  Tokens = annotate("bool __attribute__((unused)) __attribute__((const)) foo;");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_AttributeLParen);
+  EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_Unknown);
+  EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_Unknown);
+  EXPECT_TOKEN(Tokens[6], tok::r_paren, TT_AttributeRParen);
+  EXPECT_TOKEN(Tokens[8], tok::l_paren, TT_AttributeLParen);
+  EXPECT_TOKEN(Tokens[9], tok::l_paren, TT_Unknown);
+  EXPECT_TOKEN(Tokens[11], tok::r_paren, TT_Unknown);
+  EXPECT_TOKEN(Tokens[12], tok::r_paren, TT_AttributeRParen);
+  EXPECT_TOKEN(Tokens[13], tok::identifier, TT_StartOfName);
+  EXPECT_FALSE(Tokens[8]->EndsCppAttributeGroup);
+  EXPECT_TRUE(Tokens[12]->EndsCppAttributeGroup);
 
   Tokens = annotate("void __attribute__((x)) Foo();");
   ASSERT_EQ(Tokens.size(), 12u) << Tokens;

[HazardyKnusperkeks]

You either have to use a new option, or adapt the documentation. Latter clearly only refers to C++11 attributes, which __attribute__(()) isn't.

I don't think a new option is needed.

When you are already at it, you may also consider handling __declspec().

[owenca]

You either have to use a new option, or adapt the documentation. Latter clearly only refers to C++11 attributes, which __attribute__(()) isn't.

I don't think a new option is needed.

When you are already at it, you may also consider handling __declspec().

+1.