[clang-format] Add TT_CompoundRequirementLBrace for better annotation

[owenca]

Also, add ST_CompoundRequirement to help annotating */&/&& in compound requirement expressions as TT_BinaryOperator.

Fixes #121471.

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Also, add ST_CompoundRequirement so that */&/&& in compound requirement expressions are annotated as PointerOrReference.

Fixes #121471.

---

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

6 Files Affected:

• (modified) clang/lib/Format/FormatToken.h (+1)
• (modified) clang/lib/Format/TokenAnnotator.cpp (+6-1)
• (modified) clang/lib/Format/TokenAnnotator.h (+2)
• (modified) clang/lib/Format/UnwrappedLineFormatter.cpp (+1-1)
• (modified) clang/lib/Format/UnwrappedLineParser.cpp (+3-2)
• (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+12)

diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 8917049cefb865..83bed1bf7fef65 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -44,6 +44,7 @@ namespace format {
   TYPE(CastRParen)                                                             \
   TYPE(ClassLBrace)                                                            \
   TYPE(ClassRBrace)                                                            \
+  TYPE(CompoundRequirementLBrace)                                              \
   /* ternary ?: expression */                                                  \
   TYPE(ConditionalExpr)                                                        \
   /* the condition in an if statement */                                       \
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index b0f570966a63f3..cbc1b1e57d0b32 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -143,6 +143,8 @@ class AnnotatingParser {
     case TT_StructLBrace:
     case TT_UnionLBrace:
       return ST_Class;
+    case TT_CompoundRequirementLBrace:
+      return ST_CompoundRequirement;
     default:
       return ST_Other;
     }
@@ -2076,7 +2078,7 @@ class AnnotatingParser {
             TT_RecordLBrace, TT_StructLBrace, TT_UnionLBrace, TT_RequiresClause,
             TT_RequiresClauseInARequiresExpression, TT_RequiresExpression,
             TT_RequiresExpressionLParen, TT_RequiresExpressionLBrace,
-            TT_BracedListLBrace)) {
+            TT_CompoundRequirementLBrace, TT_BracedListLBrace)) {
       CurrentToken->setType(TT_Unknown);
     }
     CurrentToken->Role.reset();
@@ -3100,6 +3102,9 @@ class AnnotatingParser {
       }
     }
 
+    if (!Scopes.empty() && Scopes.back() == ST_CompoundRequirement)
+      return TT_BinaryOperator;
+
     return TT_PointerOrReference;
   }
 
diff --git a/clang/lib/Format/TokenAnnotator.h b/clang/lib/Format/TokenAnnotator.h
index 9117ca3f9fb7b5..1a250e94d97c50 100644
--- a/clang/lib/Format/TokenAnnotator.h
+++ b/clang/lib/Format/TokenAnnotator.h
@@ -40,6 +40,8 @@ enum ScopeType {
   ST_ChildBlock,
   // Contained in class declaration/definition.
   ST_Class,
+  // Contained in compound requirement.
+  ST_CompoundRequirement,
   // Contained within other scope block (function, loop, if/else, etc).
   ST_Other,
 };
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 803c600cec44db..97d3c7d0ed4eed 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -535,7 +535,7 @@ class LineJoiner {
       // Try to merge records.
       if (TheLine->Last->is(TT_EnumLBrace)) {
         ShouldMerge = Style.AllowShortEnumsOnASingleLine;
-      } else if (TheLine->Last->is(TT_RequiresExpressionLBrace)) {
+      } else if (TheLine->Last->is(TT_CompoundRequirementLBrace)) {
         ShouldMerge = Style.AllowShortCompoundRequirementOnASingleLine;
       } else if (TheLine->Last->isOneOf(TT_ClassLBrace, TT_StructLBrace)) {
         // NOTE: We use AfterClass (whereas AfterStruct exists) for both classes
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 654148a161bd7f..4912574016e70f 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -394,7 +394,7 @@ bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace,
       break;
     case tok::l_brace:
       if (InRequiresExpression) {
-        FormatTok->setFinalizedType(TT_RequiresExpressionLBrace);
+        FormatTok->setFinalizedType(TT_CompoundRequirementLBrace);
       } else if (FormatTok->Previous &&
                  FormatTok->Previous->ClosesRequiresClause) {
         // We need the 'default' case here to correctly parse a function
@@ -1702,7 +1702,8 @@ void UnwrappedLineParser::parseStructuralElement(
   }
 
   for (const bool InRequiresExpression =
-           OpeningBrace && OpeningBrace->is(TT_RequiresExpressionLBrace);
+           OpeningBrace && OpeningBrace->isOneOf(TT_RequiresExpressionLBrace,
+                                                 TT_CompoundRequirementLBrace);
        !eof();) {
     if (IsCpp && FormatTok->isCppAlternativeOperatorKeyword()) {
       if (auto *Next = Tokens->peekNextToken(/*SkipComment=*/true);
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index d61b9adf4f58c6..053b1c4059e2c9 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1456,6 +1456,18 @@ TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {
   EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, CompoundRequirement) {
+  auto Tokens = annotate("template <typename T, typename V>\n"
+                         "concept CheckMultiplicableBy = requires(T a, V b) {\n"
+                         "  { a * b } -> std::same_as<T>;\n"
+                         "};");
+  ASSERT_EQ(Tokens.size(), 36u) << Tokens;
+
+  EXPECT_TOKEN(Tokens[19], tok::l_brace, TT_RequiresExpressionLBrace);
+  EXPECT_TOKEN(Tokens[20], tok::l_brace, TT_CompoundRequirementLBrace);
+  EXPECT_TOKEN(Tokens[22], tok::star, TT_BinaryOperator);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsPragmaRegion) {
   // Everything after #pragma region should be ImplicitStringLiteral
   auto Tokens = annotate("#pragma region Foo(Bar: Hello)");

[rymiel]

This is a good fix but I think it might not be addressing the whole issue. This isn't a problem with just compound-requirement, it's the case for simple-requirements too.

This is valid:

template <typename T>
concept Multiplicable = requires(T a, T b) { a * b; };

but clang-format produces this:

template <typename T>
concept Multiplicable = requires(T a, T b) { a *b; };

Even with this patch applied.

I'm not exactly sure what clang-format is doing here and how this wasn't caught earlier. I guess it thinks the contents of a requires expression aren't an expression context when they most definitely are most of the time.

[owenca]

This is a good fix but I think it might not be addressing the whole issue. This isn't a problem with just compound-requirement, it's the case for _simple-requirement_s too.

This patch fixes compound requirements as mentioned in the commit message and reported in the GitHub issue. Can you open a new issue for the simple requirement bug you noticed? I'll have a look and see if it can be fixed in a similar way.

[rymiel]

#121675