[clang-format] Remove special logic for parsing concept definitions.

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 llvm#55898 and llvm#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 llvm#58130

Depends on D140330

Reviewed By: HazardyKnusperkeks, owenpan, MyDeveloperDay

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

Author: rymiel

clang/lib/Format/TokenAnnotator.cpp

@@ -1690,8 +1690,8 @@ class AnnotatingParser {
         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;
         }
 

clang/lib/Format/UnwrappedLineParser.cpp

@@ -1818,9 +1818,6 @@ void UnwrappedLineParser::parseStructuralElement(
         break;
       }
       break;
-    case tok::kw_concept:
-      parseConcept();
-      return;
     case tok::kw_requires: {
       if (Style.isCpp()) {
         bool ParsedClause = parseRequires();
@@ -3277,26 +3274,6 @@ void UnwrappedLineParser::parseAccessSpecifier() {
   }
 }
 
-/// \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.
@@ -3463,6 +3440,8 @@ void UnwrappedLineParser::parseRequiresClause(FormatToken *RequiresToken) {
                                       ? TT_RequiresClauseInARequiresExpression
                                       : TT_RequiresClause);
 
+  // NOTE: parseConstraintExpression is only ever called from this function.
+  // It could be inlined into here.
   parseConstraintExpression();
 
   if (!InRequiresExpression)
@@ -3496,9 +3475,8 @@ void UnwrappedLineParser::parseRequiresExpression(FormatToken *RequiresToken) {
 
 /// \brief Parses a constraint expression.
 ///
-/// This is either the definition of a concept, or the body of a requires
-/// clause. It returns, when the parsing is complete, or the expression is
-/// incorrect.
+/// This is the body of a requires 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

clang/lib/Format/UnwrappedLineParser.h

@@ -158,7 +158,6 @@ class UnwrappedLineParser {
   void parseAccessSpecifier();
   bool parseEnum();
   bool parseStructLike();
-  void parseConcept();
   bool parseRequires();
   void parseRequiresClause(FormatToken *RequiresToken);
   void parseRequiresExpression(FormatToken *RequiresToken);

clang/unittests/Format/FormatTest.cpp

@@ -23588,19 +23588,19 @@ TEST_F(FormatTest, Concepts) {
                "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(); } "
                "&& sizeof(T) <= 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 @@ TEST_F(FormatTest, Concepts) {
   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 @@ TEST_F(FormatTest, Concepts) {
   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,21 +23937,20 @@ TEST_F(FormatTest, Concepts) {
 
   // 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>;");
+  verifyNoCrash("template <typename T>\n"
+                "concept C = requires C2<T>;");
 
-  verifyFormat("template <typename T>\n"
-               "concept C = 5 + 4;");
+  verifyNoCrash("template <typename T>\n"
+                "concept C = 5 + 4;");
 
-  verifyFormat("template <typename T>\n"
-               "concept C =\n"
-               "class X;");
+  verifyNoCrash("template <typename T>\n"
+                "concept C = class X;");
 
-  verifyFormat("template <typename T>\n"
-               "concept C = [] && true;");
+  verifyNoCrash("template <typename T>\n"
+                "concept C = [] && true;");
 
-  verifyFormat("template <typename T>\n"
-               "concept C = [] && requires(T t) { typename T::size_type; };");
+  verifyNoCrash("template <typename T>\n"
+                "concept C = [] && requires(T t) { typename T::size_type; };");
 }
 
 TEST_F(FormatTest, RequiresClausesPositions) {