[clang] be9a7fd - [clang-format] Fixed handling of requires clauses followed by attributes

[Björn Schäpers via cfe-commits]

Author: Björn Schäpers
Date: 2022-02-20T22:33:26+01:00
New Revision: be9a7fdd6a8aec669bcb1f6a68087ab4a70ddb2e

URL: https://github.com/llvm/llvm-project/commit/be9a7fdd6a8aec669bcb1f6a68087ab4a70ddb2e
DIFF: https://github.com/llvm/llvm-project/commit/be9a7fdd6a8aec669bcb1f6a68087ab4a70ddb2e.diff

LOG: [clang-format] Fixed handling of requires clauses followed by attributes

Fixes https://github.com/llvm/llvm-project/issues/53820.

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

Added: 
    

Modified: 
    clang/lib/Format/UnwrappedLineParser.cpp
    clang/unittests/Format/FormatTest.cpp
    clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index c1bd45beb7b3..4c5ab5346b7d 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Support/raw_ostream.h"
 
 #include <algorithm>
+#include <utility>
 
 #define DEBUG_TYPE "format-parser"
 
@@ -3007,7 +3008,16 @@ void UnwrappedLineParser::parseRequiresExpression(FormatToken *RequiresToken) {
 /// clause. It returns, when the parsing is complete, or the expression is
 /// incorrect.
 void UnwrappedLineParser::parseConstraintExpression() {
+  // The special handling for lambdas is needed since tryToParseLambda() eats a
+  // token and if a requires expression is the last part of a requires clause
+  // and followed by an attribute like [[nodiscard]] the ClosesRequiresClause is
+  // not set on the correct token. Thus we need to be aware if we even expect a
+  // lambda to be possible.
+  // template <typename T> requires requires { ... } [[nodiscard]] ...;
+  bool LambdaNextTimeAllowed = true;
   do {
+    bool LambdaThisTimeAllowed = std::exchange(LambdaNextTimeAllowed, false);
+
     switch (FormatTok->Tok.getKind()) {
     case tok::kw_requires: {
       auto RequiresToken = FormatTok;
@@ -3021,7 +3031,7 @@ void UnwrappedLineParser::parseConstraintExpression() {
       break;
 
     case tok::l_square:
-      if (!tryToParseLambda())
+      if (!LambdaThisTimeAllowed || !tryToParseLambda())
         return;
       break;
 
@@ -3064,10 +3074,15 @@ void UnwrappedLineParser::parseConstraintExpression() {
     case tok::pipepipe:
       FormatTok->setType(TT_BinaryOperator);
       nextToken();
+      LambdaNextTimeAllowed = true;
+      break;
+
+    case tok::comma:
+    case tok::comment:
+      LambdaNextTimeAllowed = LambdaThisTimeAllowed;
+      nextToken();
       break;
 
-    case tok::kw_true:
-    case tok::kw_false:
     case tok::kw_sizeof:
     case tok::greater:
     case tok::greaterequal:
@@ -3082,11 +3097,16 @@ void UnwrappedLineParser::parseConstraintExpression() {
     case tok::minus:
     case tok::star:
     case tok::slash:
-    case tok::numeric_constant:
     case tok::kw_decltype:
-    case tok::comment:
-    case tok::comma:
+      LambdaNextTimeAllowed = true;
+      // Just eat them.
+      nextToken();
+      break;
+
+    case tok::numeric_constant:
     case tok::coloncolon:
+    case tok::kw_true:
+    case tok::kw_false:
       // Just eat them.
       nextToken();
       break;

diff  --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index f71f8dc5de45..f6810766d83d 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -23784,7 +23784,7 @@ TEST_F(FormatTest, Concepts) {
                "concept C = [] && requires(T t) { typename T::size_type; };");
 }
 
-TEST_F(FormatTest, RequiresClauses) {
+TEST_F(FormatTest, RequiresClausesPositions) {
   auto Style = getLLVMStyle();
   EXPECT_EQ(Style.RequiresClausePosition, FormatStyle::RCPS_OwnLine);
   EXPECT_EQ(Style.IndentRequiresClause, true);
@@ -24007,6 +24007,16 @@ TEST_F(FormatTest, RequiresClauses) {
                ColumnStyle);
 }
 
+TEST_F(FormatTest, RequiresClauses) {
+  verifyFormat("struct [[nodiscard]] zero_t {\n"
+               "  template <class T>\n"
+               "    requires requires { number_zero_v<T>; }\n"
+               "  [[nodiscard]] constexpr operator T() const {\n"
+               "    return number_zero_v<T>;\n"
+               "  }\n"
+               "};");
+}
+
 TEST_F(FormatTest, StatementAttributeLikeMacros) {
   FormatStyle Style = getLLVMStyle();
   StringRef Source = "void Foo::slot() {\n"

diff  --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 0abe533dd5fc..e7bc26b5a9b5 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -232,6 +232,20 @@ TEST_F(TokenAnnotatorTest, UnderstandsRequiresClausesAndConcepts) {
                     "Namespace::Outer<T>::Inner::Constant) {}");
   ASSERT_EQ(Tokens.size(), 24u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("struct [[nodiscard]] zero_t {\n"
+                    "  template<class T>\n"
+                    "    requires requires { number_zero_v<T>; }\n"
+                    "  [[nodiscard]] constexpr operator T() const { "
+                    "return number_zero_v<T>; }\n"
+                    "};");
+  ASSERT_EQ(Tokens.size(), 44u);
+  EXPECT_TOKEN(Tokens[13], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[14], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_RequiresExpressionLBrace);
+  EXPECT_TOKEN(Tokens[21], tok::r_brace, TT_Unknown);
+  EXPECT_EQ(Tokens[21]->MatchingParen, Tokens[15]);
+  EXPECT_TRUE(Tokens[21]->ClosesRequiresClause);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {
@@ -507,6 +521,35 @@ TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
   NumberOfAdditionalRequiresClauseTokens = 14u;
   NumberOfTokensBeforeRequires = 5u;
 
+  ASSERT_EQ(BaseTokens.size(), NumberOfBaseTokens) << BaseTokens;
+  ASSERT_EQ(ConstrainedTokens.size(),
+            NumberOfBaseTokens + NumberOfAdditionalRequiresClauseTokens)
+      << ConstrainedTokens;
+
+  for (auto I = 0u; I < NumberOfBaseTokens; ++I)
+    if (I < NumberOfTokensBeforeRequires)
+      EXPECT_EQ(*BaseTokens[I], *ConstrainedTokens[I]) << I;
+    else
+      EXPECT_EQ(*BaseTokens[I],
+                *ConstrainedTokens[I + NumberOfAdditionalRequiresClauseTokens])
+          << I;
+
+  BaseTokens = annotate("struct [[nodiscard]] zero_t {\n"
+                        "  template<class T>\n"
+                        "  [[nodiscard]] constexpr operator T() const { "
+                        "return number_zero_v<T>; }\n"
+                        "};");
+
+  ConstrainedTokens = annotate("struct [[nodiscard]] zero_t {\n"
+                               "  template<class T>\n"
+                               "    requires requires { number_zero_v<T>; }\n"
+                               "  [[nodiscard]] constexpr operator T() const { "
+                               "return number_zero_v<T>; }\n"
+                               "};");
+  NumberOfBaseTokens = 35u;
+  NumberOfAdditionalRequiresClauseTokens = 9u;
+  NumberOfTokensBeforeRequires = 13u;
+
   ASSERT_EQ(BaseTokens.size(), NumberOfBaseTokens) << BaseTokens;
   ASSERT_EQ(ConstrainedTokens.size(),
             NumberOfBaseTokens + NumberOfAdditionalRequiresClauseTokens)