[clang-format] Fix formatting of struct-like records followed by variable declaration.

Fixes #24781.
Fixes #38160.

This patch splits `TT_RecordLBrace` for classes/enums/structs/unions (and other records, e.g. interfaces) and uses the brace type to avoid the error-prone scanning for record token.

The mentioned bugs were provoked by the scanning being too limited (and so not considering `const` or `constexpr`, or other qualifiers, on an anonymous struct variable declaration).

Moreover, the proposed solution is more efficient as we parse tokens once only (scanning being parsing too).

Reviewed By: MyDeveloperDay, HazardyKnusperkeks

Differential Revision: https://reviews.llvm.org/D119785

Author: mkurdej

clang/lib/Format/FormatToken.h

@@ -37,6 +37,7 @@ namespace format {
   TYPE(BlockComment)                                                           \
   TYPE(BracedListLBrace)                                                       \
   TYPE(CastRParen)                                                             \
+  TYPE(ClassLBrace)                                                            \
   TYPE(CompoundRequirementLBrace)                                              \
   TYPE(ConditionalExpr)                                                        \
   TYPE(ConflictAlternative)                                                    \
@@ -47,6 +48,7 @@ namespace format {
   TYPE(DesignatedInitializerLSquare)                                           \
   TYPE(DesignatedInitializerPeriod)                                            \
   TYPE(DictLiteral)                                                            \
+  TYPE(EnumLBrace)                                                             \
   TYPE(FatArrow)                                                               \
   TYPE(ForEachMacro)                                                           \
   TYPE(FunctionAnnotationRParen)                                               \
@@ -108,6 +110,7 @@ namespace format {
   TYPE(StartOfName)                                                            \
   TYPE(StatementAttributeLikeMacro)                                            \
   TYPE(StatementMacro)                                                         \
+  TYPE(StructLBrace)                                                           \
   TYPE(StructuredBindingLSquare)                                               \
   TYPE(TemplateCloser)                                                         \
   TYPE(TemplateOpener)                                                         \
@@ -119,6 +122,7 @@ namespace format {
   TYPE(TypeDeclarationParen)                                                   \
   TYPE(TypenameMacro)                                                          \
   TYPE(UnaryOperator)                                                          \
+  TYPE(UnionLBrace)                                                            \
   TYPE(UntouchableMacroFunc)                                                   \
   TYPE(CSharpStringLiteral)                                                    \
   TYPE(CSharpNamedArgumentColon)                                               \

clang/lib/Format/TokenAnnotator.cpp

@@ -1425,11 +1425,12 @@ class AnnotatingParser {
             TT_LambdaArrow, TT_NamespaceMacro, TT_OverloadedOperator,
             TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral,
             TT_UntouchableMacroFunc, TT_StatementAttributeLikeMacro,
-            TT_FunctionLikeOrFreestandingMacro, TT_RecordLBrace,
-            TT_RequiresClause, TT_RequiresClauseInARequiresExpression,
-            TT_RequiresExpression, TT_RequiresExpressionLParen,
-            TT_RequiresExpressionLBrace, TT_BinaryOperator,
-            TT_CompoundRequirementLBrace, TT_BracedListLBrace))
+            TT_FunctionLikeOrFreestandingMacro, TT_ClassLBrace, TT_EnumLBrace,
+            TT_RecordLBrace, TT_StructLBrace, TT_UnionLBrace, TT_RequiresClause,
+            TT_RequiresClauseInARequiresExpression, TT_RequiresExpression,
+            TT_RequiresExpressionLParen, TT_RequiresExpressionLBrace,
+            TT_BinaryOperator, TT_CompoundRequirementLBrace,
+            TT_BracedListLBrace))
       CurrentToken->setType(TT_Unknown);
     CurrentToken->Role.reset();
     CurrentToken->MatchingParen = nullptr;

clang/lib/Format/UnwrappedLineFormatter.cpp

@@ -26,6 +26,11 @@ bool startsExternCBlock(const AnnotatedLine &Line) {
          NextNext && NextNext->is(tok::l_brace);
 }
 
+bool isRecordLBrace(const FormatToken &Tok) {
+  return Tok.isOneOf(TT_ClassLBrace, TT_EnumLBrace, TT_RecordLBrace,
+                     TT_StructLBrace, TT_UnionLBrace);
+}
+
 /// Tracks the indent level of \c AnnotatedLines across levels.
 ///
 /// \c nextLine must be called for each \c AnnotatedLine, after which \c
@@ -310,7 +315,7 @@ class LineJoiner {
           for (const FormatToken *RecordTok = (*J)->Last; RecordTok;
                RecordTok = RecordTok->Previous)
             if (RecordTok->is(tok::l_brace))
-              return RecordTok->is(TT_RecordLBrace);
+              return isRecordLBrace(*RecordTok);
         }
       }
 
@@ -457,27 +462,30 @@ class LineJoiner {
       }
     }
 
-    // Try to merge a block with left brace unwrapped that wasn't yet covered.
     if (TheLine->Last->is(tok::l_brace)) {
-      const FormatToken *Tok = TheLine->First;
       bool ShouldMerge = false;
-      if (Tok->is(tok::kw_typedef)) {
-        Tok = Tok->getNextNonComment();
-        assert(Tok);
-      }
-      if (Tok->isOneOf(tok::kw_class, tok::kw_struct)) {
+      // Try to merge records.
+      if (TheLine->Last->is(TT_EnumLBrace)) {
+        ShouldMerge = Style.AllowShortEnumsOnASingleLine;
+      } else if (TheLine->Last->isOneOf(TT_ClassLBrace, TT_StructLBrace)) {
+        // NOTE: We use AfterClass (whereas AfterStruct exists) for both classes
+        // and structs, but it seems that wrapping is still handled correctly
+        // elsewhere.
         ShouldMerge = !Style.BraceWrapping.AfterClass ||
                       (NextLine.First->is(tok::r_brace) &&
                        !Style.BraceWrapping.SplitEmptyRecord);
-      } else if (Tok->is(tok::kw_enum)) {
-        ShouldMerge = Style.AllowShortEnumsOnASingleLine;
       } else {
+        // Try to merge a block with left brace unwrapped that wasn't yet
+        // covered.
+        assert(!TheLine->First->isOneOf(tok::kw_class, tok::kw_enum,
+                                        tok::kw_struct));
         ShouldMerge = !Style.BraceWrapping.AfterFunction ||
                       (NextLine.First->is(tok::r_brace) &&
                        !Style.BraceWrapping.SplitEmptyFunction);
       }
       return ShouldMerge ? tryMergeSimpleBlock(I, E, Limit) : 0;
     }
+
     // Try to merge a function block with left brace wrapped.
     if (NextLine.First->is(TT_FunctionLBrace) &&
         Style.BraceWrapping.AfterFunction) {
@@ -715,6 +723,7 @@ class LineJoiner {
         const FormatToken *Next = Tok->getNextNonComment();
         return !Next || Next->is(tok::semi);
       };
+
       if (ShouldMerge()) {
         // We merge empty blocks even if the line exceeds the column limit.
         Tok->SpacesRequiredBefore = Style.SpaceInEmptyBlock ? 1 : 0;
@@ -723,18 +732,7 @@ class LineJoiner {
       } else if (Limit != 0 && !Line.startsWithNamespace() &&
                  !startsExternCBlock(Line)) {
         // We don't merge short records.
-        FormatToken *RecordTok = Line.First;
-        // Skip record modifiers.
-        while (RecordTok->Next &&
-               RecordTok->isOneOf(tok::kw_typedef, tok::kw_export,
-                                  Keywords.kw_declare, Keywords.kw_abstract,
-                                  tok::kw_default, Keywords.kw_override,
-                                  tok::kw_public, tok::kw_private,
-                                  tok::kw_protected, Keywords.kw_internal))
-          RecordTok = RecordTok->Next;
-        if (RecordTok &&
-            RecordTok->isOneOf(tok::kw_class, tok::kw_union, tok::kw_struct,
-                               Keywords.kw_interface))
+        if (isRecordLBrace(*Line.Last))
           return 0;
 
         // Check that we still have three lines and they fit into the limit.

clang/lib/Format/UnwrappedLineParser.cpp

@@ -3160,7 +3160,7 @@ bool UnwrappedLineParser::parseEnum() {
   // Just a declaration or something is wrong.
   if (FormatTok->isNot(tok::l_brace))
     return true;
-  FormatTok->setType(TT_RecordLBrace);
+  FormatTok->setType(TT_EnumLBrace);
   FormatTok->setBlockKind(BK_Block);
 
   if (Style.Language == FormatStyle::LK_Java) {
@@ -3397,8 +3397,22 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
       nextToken();
     }
   }
+
+  auto GetBraceType = [](const FormatToken &RecordTok) {
+    switch (RecordTok.Tok.getKind()) {
+    case tok::kw_class:
+      return TT_ClassLBrace;
+    case tok::kw_struct:
+      return TT_StructLBrace;
+    case tok::kw_union:
+      return TT_UnionLBrace;
+    default:
+      // Useful for e.g. interface.
+      return TT_RecordLBrace;
+    }
+  };
   if (FormatTok->Tok.is(tok::l_brace)) {
-    FormatTok->setType(TT_RecordLBrace);
+    FormatTok->setType(GetBraceType(InitialToken));
     if (ParseAsExpr) {
       parseChildBlock();
     } else {

clang/unittests/Format/FormatTest.cpp

@@ -3395,10 +3395,43 @@ TEST_F(FormatTest, BreakInheritanceStyle) {
                StyleWithInheritanceBreakAfterComma);
 }
 
-TEST_F(FormatTest, FormatsVariableDeclarationsAfterStructOrClass) {
+TEST_F(FormatTest, FormatsVariableDeclarationsAfterRecord) {
   verifyFormat("class A {\n} a, b;");
   verifyFormat("struct A {\n} a, b;");
-  verifyFormat("union A {\n} a;");
+  verifyFormat("union A {\n} a, b;");
+
+  verifyFormat("constexpr class A {\n} a, b;");
+  verifyFormat("constexpr struct A {\n} a, b;");
+  verifyFormat("constexpr union A {\n} a, b;");
+
+  verifyFormat("namespace {\nclass A {\n} a, b;\n} // namespace");
+  verifyFormat("namespace {\nstruct A {\n} a, b;\n} // namespace");
+  verifyFormat("namespace {\nunion A {\n} a, b;\n} // namespace");
+
+  verifyFormat("namespace {\nconstexpr class A {\n} a, b;\n} // namespace");
+  verifyFormat("namespace {\nconstexpr struct A {\n} a, b;\n} // namespace");
+  verifyFormat("namespace {\nconstexpr union A {\n} a, b;\n} // namespace");
+
+  verifyFormat("namespace ns {\n"
+               "class {\n"
+               "} a, b;\n"
+               "} // namespace ns");
+  verifyFormat("namespace ns {\n"
+               "const class {\n"
+               "} a, b;\n"
+               "} // namespace ns");
+  verifyFormat("namespace ns {\n"
+               "constexpr class C {\n"
+               "} a, b;\n"
+               "} // namespace ns");
+  verifyFormat("namespace ns {\n"
+               "class { /* comment */\n"
+               "} a, b;\n"
+               "} // namespace ns");
+  verifyFormat("namespace ns {\n"
+               "const class { /* comment */\n"
+               "} a, b;\n"
+               "} // namespace ns");
 }
 
 TEST_F(FormatTest, FormatsEnum) {

clang/unittests/Format/TokenAnnotatorTest.cpp

@@ -78,25 +78,38 @@ TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmpInMacroDefinition) {
 TEST_F(TokenAnnotatorTest, UnderstandsClasses) {
   auto Tokens = annotate("class C {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
-  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_ClassLBrace);
+
+  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);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsStructs) {
   auto Tokens = annotate("struct S {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
-  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_StructLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUnions) {
   auto Tokens = annotate("union U {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
-  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_UnionLBrace);
+
+  Tokens = annotate("union U { void f() { return; } };");
+  EXPECT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_UnionLBrace);
+  EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_FunctionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsEnums) {
   auto Tokens = annotate("enum E {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
-  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_EnumLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsLBracesInMacroDefinition) {