[clang] [clang-format] Make bitwise and imply requires clause (PR #110942)

[Emilia Kond via cfe-commits]

https://github.com/rymiel updated https://github.com/llvm/llvm-project/pull/110942

>From ef604d0bd621cfef32a8dd4b5410bd530ed2fe80 Mon Sep 17 00:00:00 2001
From: Emilia Kond <emilia at rymiel.space>
Date: Thu, 3 Oct 2024 03:06:10 +0300
Subject: [PATCH 1/3] [clang-format] Make some binary operations imply requires
 clause

This patch adjusts the requires clause/expression lookahead heuristic to
assume that some binary operation mean that the requires keyword refers
to a clause.

This partially reverts the removed case clauses from
9b68c095d6b52d6ec0390c653528f65c42e5f570, with an additional check that
we're not in a type parameter.

This is quite a band-aid fix, it may be worth it to rewrite that whole
heuristic to track more state in the future, instead of just blindly
marching forward across multiple unrelated definitions, since right now,
the definition following the one with the requires clause can influence
whether the heuristic chooses clause or expression.

Fixes https://github.com/llvm/llvm-project/issues/110485
---
 clang/lib/Format/UnwrappedLineParser.cpp      | 11 +++++++++++
 clang/unittests/Format/TokenAnnotatorTest.cpp | 15 +++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 40f77266fabdca..98078fb50dcfaa 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -3528,6 +3528,17 @@ bool UnwrappedLineParser::parseRequires() {
         return false;
       }
       break;
+    case tok::equalequal:
+    case tok::greaterequal:
+    case tok::lessequal:
+    case tok::r_paren:
+    case tok::pipepipe:
+      if (OpenAngles == 0) {
+        FormatTok = Tokens->setPosition(StoredPosition);
+        parseRequiresClause(RequiresToken);
+        return true;
+      }
+      break;
     case tok::eof:
       // Break out of the loop.
       Lookahead = 50;
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 1884d41a5f23f5..6f1b1df0bda655 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1296,6 +1296,21 @@ TEST_F(TokenAnnotatorTest, UnderstandsRequiresClausesAndConcepts) {
   Tokens = annotate("bool x = t && requires(Foo<C1 || C2> x) { x.foo(); };");
   ASSERT_EQ(Tokens.size(), 25u) << Tokens;
   EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresExpression);
+
+  // Second function definition is required due to lookahead
+  Tokens = annotate("void f() & requires(n == 1) {}\nvoid g();");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("void f() & requires(n || h) {}\nvoid g();");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("bool x = t && requires(F<n == 1> x) { x.foo(); };");
+  ASSERT_EQ(Tokens.size(), 25u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresExpression);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {

>From 64b9240349f07729de3700eb4a9c43fc135e38ed Mon Sep 17 00:00:00 2001
From: Emilia Kond <emilia at rymiel.space>
Date: Thu, 10 Oct 2024 16:09:43 +0300
Subject: [PATCH 2/3] Reduce scope of PR

---
 clang/lib/Format/UnwrappedLineParser.cpp | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 98078fb50dcfaa..b051994c800494 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -3470,10 +3470,10 @@ bool UnwrappedLineParser::parseRequires() {
   case tok::r_paren:
   case tok::kw_noexcept:
   case tok::kw_const:
+  case tok::amp:
     // This is a requires clause.
     parseRequiresClause(RequiresToken);
     return true;
-  case tok::amp:
   case tok::ampamp: {
     // This can be either:
     // if (... && requires (T t) ...)
@@ -3528,17 +3528,6 @@ bool UnwrappedLineParser::parseRequires() {
         return false;
       }
       break;
-    case tok::equalequal:
-    case tok::greaterequal:
-    case tok::lessequal:
-    case tok::r_paren:
-    case tok::pipepipe:
-      if (OpenAngles == 0) {
-        FormatTok = Tokens->setPosition(StoredPosition);
-        parseRequiresClause(RequiresToken);
-        return true;
-      }
-      break;
     case tok::eof:
       // Break out of the loop.
       Lookahead = 50;

>From d06b7c91a41a02b87aeb6e5e557867eae8ddadeb Mon Sep 17 00:00:00 2001
From: Emilia Kond <emilia at rymiel.space>
Date: Mon, 21 Oct 2024 23:40:54 +0300
Subject: [PATCH 3/3] clean up tests

---
 clang/unittests/Format/TokenAnnotatorTest.cpp | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 6f1b1df0bda655..fa85f32f0e79be 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1298,19 +1298,13 @@ TEST_F(TokenAnnotatorTest, UnderstandsRequiresClausesAndConcepts) {
   EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresExpression);
 
   // Second function definition is required due to lookahead
-  Tokens = annotate("void f() & requires(n == 1) {}\nvoid g();");
+  Tokens = annotate("void f() &\n"
+                    "  requires(n == 1)\n"
+                    "{}\n"
+                    "void g();");
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[4], tok::amp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresClause);
-
-  Tokens = annotate("void f() & requires(n || h) {}\nvoid g();");
-  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
-  EXPECT_TOKEN(Tokens[4], tok::amp, TT_PointerOrReference);
-  EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresClause);
-
-  Tokens = annotate("bool x = t && requires(F<n == 1> x) { x.foo(); };");
-  ASSERT_EQ(Tokens.size(), 25u) << Tokens;
-  EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresExpression);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {