[clang-format] Fix bug with parsing of function/variable names.

[gedare]

Summary

Function and variable names are not detected correctly when there is an
__attribute__((x)) preceding the name.

Fixes Github Issue #64137
https://github.com/llvm/llvm-project/issues/64137

[HazardyKnusperkeks]

Does this result in a different annotation? Could you add a test for that?

clang/lib/Format/TokenAnnotator.cpp

2249–2252

[gedare]

In D156370#4536793, @HazardyKnusperkeks wrote:

Does this result in a different annotation? Could you add a test for that?

If I understand you correctly, it does, for example:
$ echo "void __attribute__((naked)) foo(int bar);" | clang-format -style=llvm -debug 2>&1 | grep \'foo\'

Without the change:
M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=23 Name=identifier L=31 PPK=2 FakeLParens= FakeRParens=0 II=0x555d17efdce0 Text='foo'

With the change:
M=0 C=1 T=FunctionDeclarationName S=1 F=0 B=0 BK=0 P=80 Name=identifier L=31 PPK=2 FakeLParens= FakeRParens=0 II=0x559855502ce0 Text='foo'

I don't how to test for that.

[HazardyKnusperkeks]

In D156370#4537033, @gedare wrote:

In D156370#4536793, @HazardyKnusperkeks wrote:

Does this result in a different annotation? Could you add a test for that?

If I understand you correctly, it does, for example:
$ echo "void __attribute__((naked)) foo(int bar);" | clang-format -style=llvm -debug 2>&1 | grep \'foo\'

Without the change:
M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=23 Name=identifier L=31 PPK=2 FakeLParens= FakeRParens=0 II=0x555d17efdce0 Text='foo'

With the change:
M=0 C=1 T=FunctionDeclarationName S=1 F=0 B=0 BK=0 P=80 Name=identifier L=31 PPK=2 FakeLParens= FakeRParens=0 II=0x559855502ce0 Text='foo'

I don't how to test for that.

Yes that stuff. Tests are in: https://github.com/llvm/llvm-project/blob/main/clang/unittests/Format/TokenAnnotatorTest.cpp
If there is none that matches, just create a new one.

[owenpan]

In D156370#4540257, @HazardyKnusperkeks wrote:

Yes that stuff. Tests are in: https://github.com/llvm/llvm-project/blob/main/clang/unittests/Format/TokenAnnotatorTest.cpp
If there is none that matches, just create a new one.

@gedare, I think this is the only missing piece for your patch to get accepted. 🙂

[gedare]

Add TokenAnnotator tests and light refactoring.

[owenpan]

clang/lib/Format/TokenAnnotator.cpp

2248–2252

We should annotate parens separately as LParen and RParen like we recently did with braces. Then we could simply do:

if (PreviousNotConst->isOneOf(TT_TypeDeclarationRParen, TT_AttributeRParen))

clang/lib/Format/TokenAnnotator.cpp

bool IsPPKeyword = PreviousNotConst->is(tok::identifier) &&
                        PreviousNotConst->Previous->is(tok::hash);
 
     if (PreviousNotConst->is(TT_TemplateCloser)) {
       return PreviousNotConst && PreviousNotConst->MatchingParen &&
              PreviousNotConst->MatchingParen->Previous &&
              PreviousNotConst->MatchingParen->Previous->isNot(tok::period) &&
              PreviousNotConst->MatchingParen->Previous->isNot(tok::kw_template);
     }
 
-    if (PreviousNotConst->is(tok::r_paren) &&
-        PreviousNotConst->is(TT_TypeDeclarationParen)) {
+    if ((PreviousNotConst->is(tok::r_paren) &&
+         PreviousNotConst->is(TT_TypeDeclarationParen)) ||
+        PreviousNotConst->is(TT_AttributeRParen)) {
       return true;
     }
 
     // If is a preprocess keyword like #define.
     if (IsPPKeyword)
       return false;
 
     // int a or auto a.
     if (PreviousNotConst->isOneOf(tok::identifier, tok::kw_auto))

clang/unittests/Format/FormatTest.cpp

verifyFormat("switch (x) {\n"
                SpaceControlStatements);
 
   FormatStyle SpaceFuncDecl = getLLVMStyle();
   SpaceFuncDecl.SpaceBeforeParens = FormatStyle::SBPO_Custom;
   SpaceFuncDecl.SpaceBeforeParensOptions.AfterFunctionDeclarationName = true;
 
   verifyFormat("int f ();", SpaceFuncDecl);
   verifyFormat("void f(int a, T b) {}", SpaceFuncDecl);
+  verifyFormat("void __attribute__((asdf)) f(int a, T b) {}", SpaceFuncDecl);
   verifyFormat("A::A() : a(1) {}", SpaceFuncDecl);
   verifyFormat("void f () __attribute__((asdf));", SpaceFuncDecl);
+  verifyFormat("void __attribute__((asdf)) f ();", SpaceFuncDecl);
   verifyFormat("#define A(x) x", SpaceFuncDecl);
   verifyFormat("#define A (x) x", SpaceFuncDecl);
   verifyFormat("#if defined(x)\n"
                "#endif",
                SpaceFuncDecl);
   verifyFormat("auto i = std::make_unique<int>(5);", SpaceFuncDecl);
   verifyFormat("size_t x = sizeof(x);", SpaceFuncDecl);
   verifyFormat("auto f (int x) -> decltype(x);", SpaceFuncDecl);
verifyFormat("M(std::size_t R, std::size_t C) : C(C), data(R) {}",
                SpaceFuncDecl);
 
   FormatStyle SpaceFuncDef = getLLVMStyle();
   SpaceFuncDef.SpaceBeforeParens = FormatStyle::SBPO_Custom;
   SpaceFuncDef.SpaceBeforeParensOptions.AfterFunctionDefinitionName = true;
 
   verifyFormat("int f();", SpaceFuncDef);
   verifyFormat("void f (int a, T b) {}", SpaceFuncDef);
+  verifyFormat("void __attribute__((asdf)) f (int a, T b) {}", SpaceFuncDef);
   verifyFormat("A::A() : a(1) {}", SpaceFuncDef);
   verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
+  verifyFormat("void __attribute__((asdf)) f();", SpaceFuncDef);
   verifyFormat("#define A(x) x", SpaceFuncDef);
   verifyFormat("#define A (x) x", SpaceFuncDef);
   verifyFormat("#if defined(x)\n"
                "#endif",
                SpaceFuncDef);
   verifyFormat("auto i = std::make_unique<int>(5);", SpaceFuncDef);
   verifyFormat("size_t x = sizeof(x);", SpaceFuncDef);
   verifyFormat("auto f(int x) -> decltype(x);", SpaceFuncDef);

clang/unittests/Format/TokenAnnotatorTest.cpp

TEST_F(TokenAnnotatorTest, UnderstandsJavaScript) {
   EXPECT_BRACE_KIND(Tokens[3], BK_Unknown);
   EXPECT_BRACE_KIND(Tokens[11], BK_Unknown);
   EXPECT_TOKEN(Tokens[11], tok::r_brace, TT_Unknown);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsAttributes) {
   auto Tokens = annotate("bool foo __attribute__((unused));");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::identifier, TT_StartOfName);
   EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_AttributeLParen);
   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);
 
   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);
 
+  Tokens = annotate("bool __attribute__((unused)) foo;");
+  ASSERT_EQ(Tokens.size(), 10u) << 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[7], tok::identifier, TT_StartOfName);
+
+  Tokens = annotate("void __attribute__((x)) Foo();");
+  ASSERT_EQ(Tokens.size(), 12u) << 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[7], tok::identifier, TT_FunctionDeclarationName);
+
   FormatStyle Style = getLLVMStyle();
   Style.AttributeMacros.push_back("FOO");
   Tokens = annotate("bool foo FOO(unused);", Style);
   ASSERT_EQ(Tokens.size(), 8u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::identifier, TT_AttributeMacro);
   EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_AttributeLParen);
   EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_AttributeRParen);
 }