rymiel created this revision.
rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Previously, clang-format relied on a special method to parse concept
definitions, `UnwrappedLineParser::parseConcept()`, which deferred to
`UnwrappedLineParser::parseConstraintExpression()`. This is problematic,
because the C++ grammar treats concepts and requires clauses
differently, causing issues such as 
https://github.com/llvm/llvm-project/issues/55898 and 
https://github.com/llvm/llvm-project/issues/58130.

This patch removes `parseConcept`, letting the formatter parse concept
definitions as more like what they actually are, fancy bool definitions.

NOTE that because of this, some long concept definitions change in their
formatting, as can be seen in the changed tests. This is because of a
change in split penalties, caused by a change in MightBeFunctionDecl on
the concept definition line, which was previously `true` but with this
patch is now `false`.

One might argue that `false` is a more "correct" value for concept
definitions, but I'd be fine with setting it to `true` again to maintain
compatibility with previous versions.

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

Depends on D140330 <https://reviews.llvm.org/D140330>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140339

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23588,10 +23588,10 @@
                "concept DelayedCheck = !!false || requires(T t) { t.bar(); } "
                "&& sizeof(T) <= 8;");
 
-  verifyFormat(
-      "template <typename T>\n"
-      "concept DelayedCheck = static_cast<bool>(0) ||\n"
-      "                       requires(T t) { t.bar(); } && sizeof(T) <= 8;");
+  verifyFormat("template <typename T>\n"
+               "concept DelayedCheck =\n"
+               "    static_cast<bool>(0) || requires(T t) { t.bar(); } && "
+               "sizeof(T) <= 8;");
 
   verifyFormat("template <typename T>\n"
                "concept DelayedCheck = bool(0) || requires(T t) { t.bar(); } "
@@ -23599,8 +23599,8 @@
 
   verifyFormat(
       "template <typename T>\n"
-      "concept DelayedCheck = (bool)(0) ||\n"
-      "                       requires(T t) { t.bar(); } && sizeof(T) <= 8;");
+      "concept DelayedCheck =\n"
+      "    (bool)(0) || requires(T t) { t.bar(); } && sizeof(T) <= 8;");
 
   verifyFormat("template <typename T>\n"
                "concept DelayedCheck = (bool)0 || requires(T t) { t.bar(); } "
@@ -23636,6 +23636,9 @@
   verifyFormat("template <typename T>\n"
                "concept True = S<T>::Value;");
 
+  verifyFormat("template <S T>\n"
+               "concept True = T.field;");
+
   verifyFormat(
       "template <typename T>\n"
       "concept C = []() { return true; }() && requires(T t) { t.bar(); } &&\n"
@@ -23914,11 +23917,9 @@
   verifyFormat("template <typename T> concept True = true;", Style);
 
   verifyFormat(
-      "template <typename T> concept C = decltype([]() -> std::true_type {\n"
-      "                                    return {};\n"
-      "                                  }())::value &&\n"
-      "                                  requires(T t) { t.bar(); } && "
-      "sizeof(T) <= 8;",
+      "template <typename T> concept C =\n"
+      "    decltype([]() -> std::true_type { return {}; }())::value &&\n"
+      "    requires(T t) { t.bar(); } && sizeof(T) <= 8;",
       Style);
 
   verifyFormat("template <typename T> concept Semiregular =\n"
@@ -23936,15 +23937,15 @@
 
   // The following tests are invalid C++, we just want to make sure we don't
   // assert.
-  verifyFormat("template <typename T>\n"
-               "concept C = requires C2<T>;");
-
-  verifyFormat("template <typename T>\n"
-               "concept C = 5 + 4;");
-
   verifyFormat("template <typename T>\n"
                "concept C =\n"
-               "class X;");
+               "  requires C2<T>;");
+
+  verifyFormat("template <typename T>\n"
+               "concept C = 5 + 4;");
+
+  verifyFormat("template <typename T>\n"
+               "concept C = class X;");
 
   verifyFormat("template <typename T>\n"
                "concept C = [] && true;");
Index: clang/lib/Format/UnwrappedLineParser.h
===================================================================
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -158,7 +158,6 @@
   void parseAccessSpecifier();
   bool parseEnum();
   bool parseStructLike();
-  void parseConcept();
   bool parseRequires();
   void parseRequiresClause(FormatToken *RequiresToken);
   void parseRequiresExpression(FormatToken *RequiresToken);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1813,9 +1813,6 @@
         break;
       }
       break;
-    case tok::kw_concept:
-      parseConcept();
-      return;
     case tok::kw_requires: {
       if (Style.isCpp()) {
         bool ParsedClause = parseRequires();
@@ -3272,26 +3269,6 @@
   }
 }
 
-/// \brief Parses a concept definition.
-/// \pre The current token has to be the concept keyword.
-///
-/// Returns if either the concept has been completely parsed, or if it detects
-/// that the concept definition is incorrect.
-void UnwrappedLineParser::parseConcept() {
-  assert(FormatTok->is(tok::kw_concept) && "'concept' expected");
-  nextToken();
-  if (!FormatTok->is(tok::identifier))
-    return;
-  nextToken();
-  if (!FormatTok->is(tok::equal))
-    return;
-  nextToken();
-  parseConstraintExpression();
-  if (FormatTok->is(tok::semi))
-    nextToken();
-  addUnwrappedLine();
-}
-
 /// \brief Parses a requires, decides if it is a clause or an expression.
 /// \pre The current token has to be the requires keyword.
 /// \returns true if it parsed a clause.
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1680,8 +1680,8 @@
         if (!Tok)
           return false;
 
-        if (Tok->isOneOf(tok::kw_class, tok::kw_enum, tok::kw_concept,
-                         tok::kw_struct, tok::kw_using)) {
+        if (Tok->isOneOf(tok::kw_class, tok::kw_enum, tok::kw_struct,
+                         tok::kw_using)) {
           return false;
         }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to