[clang] [clang-format] Fix formatting with consecutive requires clauses (PR #220792)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-format

Author: Abdul Mohammad (abdulm5)

<details>
<summary>Changes</summary>

## Summary

- stop expression parsing at the end of each requires clause
- prevent fake-parenthesis metadata from a compound constraint from leaking into a following template declaration
- add formatter and token-annotation regressions for consecutive constrained template declarations

Fixes #<!-- -->219801.

## Testing

- built `clang-format` and `FormatTests` from a clean upstream `main` worktree
- focused requires-clause tests: 5 passed
- complete `FormatTests` suite: 1,282 passed
- verified the issue reproducer with LLVM style
- `git-clang-format`: no changes

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


4 Files Affected:

- (modified) clang/docs/ReleaseNotes.md (+2) 
- (modified) clang/lib/Format/TokenAnnotator.cpp (+23-7) 
- (modified) clang/unittests/Format/FormatTest.cpp (+7) 
- (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+14) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index 4c2bf55f6ebdd..782eb5602f886 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -741,6 +741,8 @@ features cannot lower the translation-unit ABI level;
 - Add `AfterRequiresExpression` sub-option of `BraceWrapping` to wrap the
   body of requires expressions. It is enabled by the `Allman`, `Whitesmiths`,
   and `GNU` styles of `BreakBeforeBraces`.
+- Fixed incorrect indentation of consecutive template declarations with
+  requires clauses. (#GH219801)
 
 - `QualifierOrder` now supports `typedef`, `consteval`, `constinit`,
   `thread_local`, `extern`, `mutable`, `signed`, `unsigned`, `long`, `short`,
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 105c98e704c3b..6d41074c76839 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3259,14 +3259,17 @@ class ExpressionParser {
   void parse(int Precedence = 0) {
     // Skip 'return' and ObjC selector colons as they are not part of a binary
     // expression.
-    while (Current && (Current->is(tok::kw_return) ||
-                       (Current->is(tok::colon) &&
-                        Current->isOneOf(TT_ObjCMethodExpr, TT_DictLiteral)))) {
+    while (Current && Current != RequiresClauseLimit &&
+           (Current->is(tok::kw_return) ||
+            (Current->is(tok::colon) &&
+             Current->isOneOf(TT_ObjCMethodExpr, TT_DictLiteral)))) {
       next();
     }
 
-    if (!Current || Precedence > PrecedenceArrowAndPeriod)
+    if (!Current || Current == RequiresClauseLimit ||
+        Precedence > PrecedenceArrowAndPeriod) {
       return;
+    }
 
     // Conditional expressions need to be parsed separately for proper nesting.
     if (Precedence == prec::Conditional) {
@@ -3287,7 +3290,7 @@ class ExpressionParser {
     // The first name of the current type in a port list.
     FormatToken *VerilogFirstOfType = nullptr;
 
-    while (Current) {
+    while (Current && Current != RequiresClauseLimit) {
       // In Verilog ports in a module header that don't have a type take the
       // type of the previous one.  For example,
       //   module a(output b,
@@ -3301,6 +3304,8 @@ class ExpressionParser {
 
       // Consume operators with higher precedence.
       parse(Precedence + 1);
+      if (!Current || Current == RequiresClauseLimit)
+        break;
 
       int CurrentPrecedence = getCurrentPrecedence();
       if (CurrentPrecedence > prec::Conditional &&
@@ -3351,9 +3356,18 @@ class ExpressionParser {
       // Consume scopes: (), [], <> and {}
       // In addition to that we handle require clauses as scope, so that the
       // constraints in that are correctly indented.
-      if (Current->opensScope() ||
-          Current->isOneOf(TT_RequiresClause,
+      if (Current->isOneOf(TT_RequiresClause,
                            TT_RequiresClauseInARequiresExpression)) {
+        const auto *End = Current;
+        while (End && !End->ClosesRequiresClause)
+          End = End->Next;
+
+        const auto *PreviousLimit = RequiresClauseLimit;
+        RequiresClauseLimit = End ? End->getNextNonComment() : PreviousLimit;
+        next();
+        parse();
+        RequiresClauseLimit = PreviousLimit;
+      } else if (Current->opensScope()) {
         // In fragment of a JavaScript template string can look like '}..${' and
         // thus close a scope and open a new one at the same time.
         while (Current && (!Current->closesScope() || Current->opensScope())) {
@@ -3657,6 +3671,8 @@ class ExpressionParser {
   const AdditionalKeywords &Keywords;
   const AnnotatedLine &Line;
   FormatToken *Current;
+  // The first non-comment token after the requires clause being parsed.
+  const FormatToken *RequiresClauseLimit = nullptr;
 };
 
 } // end anonymous namespace
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 5aed37aa56d52..2145bcc0edeee 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -24179,6 +24179,13 @@ TEST_F(FormatTest, RequiresClausesPositions) {
   // when the default was REI_Keyword.
   Style.RequiresExpressionIndentation = FormatStyle::REI_Keyword;
 
+  verifyFormat("template <int N, int C>\n"
+               "  requires(N > 2) && (C > 0)\n"
+               "template <typename U>\n"
+               "  requires(sizeof(U) > 0)\n"
+               "void S<N, C>::f(U) {}",
+               Style);
+
   verifyFormat("template <typename T>\n"
                "  requires(Foo<T> && std::trait<T>)\n"
                "struct Bar;",
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index b71147aaf1bc2..5dca71507a6e5 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1949,6 +1949,20 @@ TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
   RequiresTokenCount = 4;
   PrefixTokenCount = 5;
   TestRequires(__LINE__);
+
+  BaseCode = "template<typename T>\n"
+             "template<typename U>\n"
+             "  requires Bar<U>\n"
+             "void S<T>::f(U) {}";
+  ConstrainedCode = "template<typename T>\n"
+                    "  requires Foo<T> && Baz<T>\n"
+                    "template<typename U>\n"
+                    "  requires Bar<U>\n"
+                    "void S<T>::f(U) {}";
+  BaseTokenCount = 28;
+  RequiresTokenCount = 10;
+  PrefixTokenCount = 5;
+  TestRequires(__LINE__);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsAsm) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/220792