[clang-format] Handle builtins in constraint expression

[HazardyKnusperkeks]

Summary

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

[curdeius]

clang/unittests/Format/FormatTest.cpp

23814

How about other kinds?
This doesn't seem to work for at least ARRAY_TYPE_TRAIT (e.g. concept OneDimensionalArray = __array_rank(T) == 1; and EXPRESSION_TRAIT.

Is there a way to be more generic and treat all identifier-like entities like identifiers?

23814

I'd like to see a test for ALIASes as well, e.g. __is_same_as.

[curdeius]

More failing cases: __alignof, alignof (two different categories)

[HazardyKnusperkeks]

clang/unittests/Format/FormatTest.cpp

23814

How about other kinds?
This doesn't seem to work for at least ARRAY_TYPE_TRAIT (e.g. concept OneDimensionalArray = __array_rank(T) == 1; and EXPRESSION_TRAIT.

I was conservative, since I don't know what these kind of traits are. But sure can add.

Is there a way to be more generic and treat all identifier-like entities like identifiers?

Not that I know of. There is no such function in TokenKind.h.

[HazardyKnusperkeks]

More Traits.

[HazardyKnusperkeks]

clang/unittests/Format/FormatTest.cpp

23814

I have no idea how to handle the aliases, since there are many, and most are no traits, not correct here. We could add __is_same_as, as it seems to be the only trait alias, directly.

[curdeius]

Thanks for handling other traits! LGTM % alias test.

clang/unittests/Format/FormatTest.cpp

23814

I don't think you need to treat them specially, they should behave as their aliased entity actually. But please test at least one.

[owenpan]

clang/lib/Format/UnwrappedLineParser.cpp

3137–3140

Instead, you can fold case tok::identifier: below into default: like this:

default:
  if (!FormatTok->Tok.getIdentifierInfo())
    return;

  // We need to differentiate identifiers for a template deduction guide,
  ...

[HazardyKnusperkeks]

clang/lib/Format/UnwrappedLineParser.cpp

3137–3140

Thanks.

[owenpan]

clang/lib/Format/UnwrappedLineParser.cpp

3210

Do we still need this line?

3234

[cjdb]

Thanks for working on this!

clang/lib/Format/UnwrappedLineParser.cpp

3210

I'd prefer to get rid of default if we can.

[owenpan]

clang/lib/Format/UnwrappedLineParser.cpp

3210

We can't. It's the whole point of the fix.

[HazardyKnusperkeks]

clang/lib/Format/UnwrappedLineParser.cpp

3210

Do we still need this line?

No we don't. But I'd like to make it explicit that we handle identifiers here.

3210

I'd prefer to get rid of default if we can.

3234

Can do, but this is how it is used anywhere else in the code.

[owenpan]

clang/lib/Format/UnwrappedLineParser.cpp

3210

Can we use a comment instead?

3234

Yeah. Please keep the comment then.

clang/lib/Format/UnwrappedLineParser.cpp

case tok::l_paren:
       parseParens(/*AmpAmpTokenType=*/TT_BinaryOperator);
       break;
 
     case tok::l_square:
       if (!LambdaThisTimeAllowed || !tryToParseLambda())
         return;
       break;
 
-    case tok::identifier:
-      // We need to differentiate identifiers for a template deduction guide,
-      // variables, or function return types (the constraint expression has
-      // ended before that), and basically all other cases. But it's easier to
-      // check the other way around.
-      assert(FormatTok->Previous);
-      switch (FormatTok->Previous->Tok.getKind()) {
-      case tok::coloncolon:  // Nested identifier.
-      case tok::ampamp:      // Start of a function or variable for the
-      case tok::pipepipe:    // constraint expression.
-      case tok::kw_requires: // Initial identifier of a requires clause.
-      case tok::equal:       // Initial identifier of a concept declaration.
-        break;
-      default:
-        return;
-      }
-
-      // Read identifier with optional template declaration.
-      nextToken();
-      if (FormatTok->is(tok::less))
-        parseBracedList(/*ContinueOnSemicolons=*/false, /*IsEnum=*/false,
-                        /*ClosingBraceKind=*/tok::greater);
-      break;
-
     case tok::kw_const:
     case tok::semi:
     case tok::kw_class:
     case tok::kw_struct:
     case tok::kw_union:
       return;
 
     case tok::l_brace:
       // Potential function body.
       return;
 
     case tok::ampamp:
     case tok::pipepipe:
       FormatTok->setFinalizedType(TT_BinaryOperator);
case tok::kw_bool:
       // and bool is the only type, all other types as cast must be inside a
       // cast to bool an thus are handled by the other cases.
       nextToken();
       if (FormatTok->isNot(tok::l_paren))
         return;
       parseParens();
       break;
 
     default:
+      if (!FormatTok->Tok.getIdentifierInfo()) {
+        // Identifiers are part of the default case, we check for more then
+        // tok::identifier to handle builtin type traits.
         return;
       }
+
+      // We need to differentiate identifiers for a template deduction guide,
+      // variables, or function return types (the constraint expression has
+      // ended before that), and basically all other cases. But it's easier to
+      // check the other way around.
+      assert(FormatTok->Previous);
+      switch (FormatTok->Previous->Tok.getKind()) {
+      case tok::coloncolon:  // Nested identifier.
+      case tok::ampamp:      // Start of a function or variable for the
+      case tok::pipepipe:    // constraint expression.
+      case tok::kw_requires: // Initial identifier of a requires clause.
+      case tok::equal:       // Initial identifier of a concept declaration.
+        break;
+      default:
+        return;
+      }
+
+      // Read identifier with optional template declaration.
+      nextToken();
+      if (FormatTok->is(tok::less))
+        parseBracedList(/*ContinueOnSemicolons=*/false, /*IsEnum=*/false,
+                        /*ClosingBraceKind=*/tok::greater);
+      break;
+    }
   } while (!eof());
 }
 
 bool UnwrappedLineParser::parseEnum() {
   const FormatToken &InitialToken = *FormatTok;
 
   // Won't be 'enum' for NS_ENUMs.
   if (FormatTok->is(tok::kw_enum))

clang/unittests/Format/FormatTest.cpp

verifyFormat("template <typename T, typename U>\n"
                "void doFoo(FooableWith<int> auto t) { t.foo(3); }");
 
   verifyFormat("template <typename T>\n"
                "concept Context = is_specialization_of_v<context, T>;");
 
   verifyFormat("template <typename T>\n"
                "concept Node = std::is_object_v<T>;");
 
+  verifyFormat("template <class T>\n"
+               "concept integral = __is_integral(T);");
+
+  verifyFormat("template <class T>\n"
+               "concept is2D = __array_extent(T, 1) == 2;");
+
+  verifyFormat("template <class T>\n"
+               "concept isRhs = __is_rvalue_expr(std::declval<T>() + 2)");
+
+  verifyFormat("template <class T, class T2>\n"
+               "concept Same = __is_same_as<T, T2>;");
+
   auto Style = getLLVMStyle();
   Style.BreakBeforeConceptDeclarations = FormatStyle::BBCDS_Allowed;
 
   verifyFormat(
       "template <typename T>\n"
       "concept C = requires(T t) {\n"
       "              requires Bar<T> && Foo<T>;\n"
       "              requires((trait<T> && Baz) || (T2<T> && Foo<T>));\n"