[clang-format] Don't annotate left brace of class as FunctionLBrace

[owenpan]

Summary

The l_brace of class/struct/union was incorrectly annotated as TT_FunctionLBrace in the presence of attributes. This in turn would cause the RemoveSemicolon option to remove the semicolon at the end of the declaration, resulting in invalid code being generated.

Fixes https://github.com/llvm/llvm-project/issues/61188.

[MyDeveloperDay]

Thanks for this, LGTM (maybe we can cherry pick this to 16.0)

[HazardyKnusperkeks]

In D145344#4171087, @MyDeveloperDay wrote:

Thanks for this, LGTM (maybe we can cherry pick this to 16.0)

+1 because of the invalid code.

[MyDeveloperDay]

The upshot of this is better TT_FunctionLBrace/TT_ClassLBrace detection, which is great becuase if a class { or function { is incorrectly detected we are then in unknown territory as to how it might become formatted, since you introduced these Annotator tests, I fell we are really helping to lock down the other rules. I would hazard a guess that this actually solves more issues than we realize.

[owenpan]

In D145344#4174634, @MyDeveloperDay wrote:

I would hazard a guess that this actually solves more issues than we realize.

That would be icing on the cake. :)

clang/lib/Format/UnwrappedLineParser.cpp

void UnwrappedLineParser::parseJavaEnumBody() {
   nextToken();
   --Line->Level;
   addUnwrappedLine();
 }
 
 void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
   const FormatToken &InitialToken = *FormatTok;
   nextToken();
+  handleAttributes();
 
   // The actual identifier can be a nested name specifier, and in macros
   // it is often token-pasted.
-  // An [[attribute]] can be before the identifier.
   while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::hashhash,
                             tok::kw___attribute, tok::kw___declspec,
-                            tok::kw_alignas, tok::l_square, tok::r_square) ||
+                            tok::kw_alignas) ||
          ((Style.Language == FormatStyle::LK_Java || Style.isJavaScript()) &&
           FormatTok->isOneOf(tok::period, tok::comma))) {
     if (Style.isJavaScript() &&
         FormatTok->isOneOf(Keywords.kw_extends, Keywords.kw_implements)) {
       // JavaScript/TypeScript supports inline object types in
       // extends/implements positions:
       //     class Foo implements {bar: number} { }
       nextToken();
       if (FormatTok->is(tok::l_brace)) {
         tryToParseBracedList();
         continue;
       }
     }
     bool IsNonMacroIdentifier =
         FormatTok->is(tok::identifier) &&
         FormatTok->TokenText != FormatTok->TokenText.upper();
     nextToken();
-    // We can have macros or attributes in between 'class' and the class name.
+    // We can have macros in between 'class' and the class name.
     if (!IsNonMacroIdentifier) {
       if (FormatTok->is(tok::l_paren)) {
         parseParens();
-      } else if (FormatTok->is(TT_AttributeSquare)) {
-        parseSquare();
-        // Consume the closing TT_AttributeSquare.
-        if (FormatTok->Next && FormatTok->is(TT_AttributeSquare))
-          nextToken();
       }
     }
   }
 
   // Note that parsing away template declarations here leads to incorrectly
   // accepting function declarations as record declarations.
   // In general, we cannot solve this problem. Consider:
   // class A<int> B() {}

clang/unittests/Format/FormatTest.cpp

verifyFormat("class Foo {\n"
                "  int getSomething() const { return something; };;\n"
                "};",
                Style);
 
   verifyFormat("for (;;) {\n"
                "}",
                Style);
 
+  verifyFormat("class [[deprecated(\"\")]] C {\n"
+               "  int i;\n"
+               "};",
+               Style);
+
   verifyIncompleteFormat("class C final [[deprecated(l]] {});", Style);
 
   // These tests are here to show a problem that may not be easily
   // solved, our implementation to remove semicolons is only as good
   // as our FunctionLBrace detection and this fails for empty braces
   // because we can't distringuish this from a bracelist.
   // We will enable when that is resolved.
 #if 0

clang/unittests/Format/TokenAnnotatorTest.cpp

TEST_F(TokenAnnotatorTest, UnderstandsClasses) {
 
   Tokens = annotate("const class C {} c;");
   EXPECT_EQ(Tokens.size(), 8u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::l_brace, TT_ClassLBrace);
 
   Tokens = annotate("const class {} c;");
   EXPECT_EQ(Tokens.size(), 7u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_ClassLBrace);
+
+  Tokens = annotate("class [[deprecated(\"\")]] C { int i; };");
+  EXPECT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::l_brace, TT_ClassLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsStructs) {
   auto Tokens = annotate("struct S {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_StructLBrace);
 }