[clang] [clang-format] Fix requires misannotation with comma (PR #65908)

Emilia Kond via cfe-commits cfe-commits at lists.llvm.org
Sun Sep 10 13:28:49 PDT 2023


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

>From 0cfeaee5ec5c431bc15fb037f2a20c944c508c2a Mon Sep 17 00:00:00 2001
From: Emilia Kond <emilia at rymiel.space>
Date: Sun, 10 Sep 2023 23:01:39 +0300
Subject: [PATCH 1/2] [clang-format] Fix requires misannotation with comma

clang-format uses a heuristic to determine if a requires() is either a
requires clause or requires expression, based on what is in the
parentheses. Part of this heuristic assumed that a requires clause can
never contain a comma, however this is not the case if said comma is in
the template argument of a type.

This patch allows commas to appear in a requires clause if an angle
bracket `<` has been opened.

Fixes https://github.com/llvm/llvm-project/issues/65904
---
 clang/lib/Format/UnwrappedLineParser.cpp      | 10 +++++++---
 clang/unittests/Format/TokenAnnotatorTest.cpp |  8 ++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 0ff0656a92d7222..7af9a0cba221458 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -3369,9 +3369,13 @@ bool clang::format::UnwrappedLineParser::parseRequires() {
     case tok::kw_volatile:
     case tok::kw_const:
     case tok::comma:
-      FormatTok = Tokens->setPosition(StoredPosition);
-      parseRequiresExpression(RequiresToken);
-      return false;
+      if (OpenAngles == 0) {
+        FormatTok = Tokens->setPosition(StoredPosition);
+        parseRequiresExpression(RequiresToken);
+        return false;
+      } else {
+        break;
+      }
     case tok::r_paren:
     case tok::pipepipe:
       FormatTok = Tokens->setPosition(StoredPosition);
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 434852983712940..f3cdb8a8b2cc1b4 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1016,6 +1016,14 @@ TEST_F(TokenAnnotatorTest, UnderstandsRequiresClausesAndConcepts) {
   ASSERT_EQ(Tokens.size(), 22u) << Tokens;
   EXPECT_TOKEN(Tokens[4], tok::kw_requires, TT_RequiresClause);
   EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("void f() & requires(true) {}");
+  EXPECT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("void f() & requires(C<true, true>) {}");
+  EXPECT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {

>From 7dc5321cf79d3b59546a45040c9434dff3076844 Mon Sep 17 00:00:00 2001
From: Emilia Kond <emilia at rymiel.space>
Date: Sun, 10 Sep 2023 23:27:16 +0300
Subject: [PATCH 2/2] Adjust tests

---
 clang/lib/Format/UnwrappedLineParser.cpp      |  3 +--
 clang/unittests/Format/TokenAnnotatorTest.cpp | 10 ++++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 7af9a0cba221458..c88e081936094bd 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -3373,9 +3373,8 @@ bool clang::format::UnwrappedLineParser::parseRequires() {
         FormatTok = Tokens->setPosition(StoredPosition);
         parseRequiresExpression(RequiresToken);
         return false;
-      } else {
-        break;
       }
+      break;
     case tok::r_paren:
     case tok::pipepipe:
       FormatTok = Tokens->setPosition(StoredPosition);
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index f3cdb8a8b2cc1b4..b1e14925281546c 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1019,10 +1019,12 @@ TEST_F(TokenAnnotatorTest, UnderstandsRequiresClausesAndConcepts) {
 
   Tokens = annotate("void f() & requires(true) {}");
   EXPECT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresClause);
   EXPECT_TOKEN(Tokens[4], tok::amp, TT_PointerOrReference);
 
   Tokens = annotate("void f() & requires(C<true, true>) {}");
   EXPECT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresClause);
   EXPECT_TOKEN(Tokens[4], tok::amp, TT_PointerOrReference);
 }
 
@@ -1143,6 +1145,14 @@ TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {
   EXPECT_TOKEN(Tokens[3], tok::kw_requires, TT_RequiresExpression);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_RequiresExpressionLParen);
   EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
+
+  Tokens = annotate("bool foo = requires(C<true, true> c) {\n"
+                    "  { c.foo(); }\n"
+                    "}");
+  ASSERT_EQ(Tokens.size(), 24u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsPragmaRegion) {



More information about the cfe-commits mailing list