[clang-format] Extend SpaceBeforeParens for requires

[HazardyKnusperkeks]

Summary

We can now configure the space between requires and the following paren,separate for clauses and expressions.

[curdeius]

clang/docs/ClangFormatStyleOptions.rst

3756

You meant "if there is one", right?

clang/unittests/Format/FormatTest.cpp

14306

I'd like to see some tests for mixed true/false, false/true values of these parameters. You only test false/false and true/true combinations so far.

22636

This test seems redundant. Does it test something else than the above added ones?

[HazardyKnusperkeks]

Addressed comments.

clang/docs/ClangFormatStyleOptions.rst

3756

Yeah

clang/unittests/Format/FormatTest.cpp

22636

No it doesn't, I just thought I would put the requires stuff into the requires test case. I for one work with the gtest filter on single test cases if I work on some issue.

[HazardyKnusperkeks]

Now the new diff.

[MyDeveloperDay]

LGTM (nit the clang-format check)

[owenpan]

clang/lib/Format/TokenAnnotator.cpp

3145–3148

Nit: remove else.

[HazardyKnusperkeks]

Delayed until D113319 is resolved.

[• Quuxplusone]

clang/docs/ClangFormatStyleOptions.rst

3756

IMO these options should be named InRequiresClause and InRequiresExpression: that's where the space is going. The space doesn't go after the requires-clause. The space doesn't go after the requires-expression.
It occurs to me that the name of this option could be analogous to the name of the option that controls []() {} versus [] () {}... except that it looks like there is no such option (and I'm happy about that). :) Also, the name of that option would probably just be AfterCaptureList, which doesn't help us in this case.

clang/include/clang/Format/Format.h

3371

Here and line 3380, and possibly elsewhere: s/parentheses/parenthesis/

clang/unittests/Format/FormatTest.cpp

14369

Throughout, I'd like to see simpler test cases and more of them. E.g. I think this would suffice, in lieu of lines 14305–14369:

SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresClause = true;
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresExpression = true;
verifyFormat("void f(auto x) requires (requires (int i) { x+i; }) { }", SpaceAfterRequires);
verifyFormat("if (requires (int i) { x+i; }) return;", SpaceAfterRequires);
verifyFormat("bool b = requires (int i) { x+i; };", SpaceAfterRequires);

SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresClause = true;
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresExpression = false;
verifyFormat("void f(auto x) requires (requires(int i) { x+i; }) { }", SpaceAfterRequires);
verifyFormat("if (requires(int i) { x+i; }) return;", SpaceAfterRequires);
verifyFormat("bool b = requires(int i) { x+i; };", SpaceAfterRequires);

SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresClause = false;
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresExpression = true;
verifyFormat("void f(auto x) requires(requires (int i) { x+i; }) { }", SpaceAfterRequires);
verifyFormat("if (requires (int i) { x+i; }) return;", SpaceAfterRequires);
verifyFormat("bool b = requires (int i) { x+i; };", SpaceAfterRequires);

SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresClause = false;
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresExpression = false;
verifyFormat("void f(auto x) requires(requires(int i) { x+i; }) { }", SpaceAfterRequires);
verifyFormat("if (requires(int i) { x+i; }) return;", SpaceAfterRequires);
verifyFormat("bool b = requires(int i) { x+i; };", SpaceAfterRequires);

[HazardyKnusperkeks]

clang/docs/ClangFormatStyleOptions.rst

3756

I get your point, but the space does not go anywhere in the clause/expression, so AfterRequiresForClauses?

[• Quuxplusone]

clang/docs/ClangFormatStyleOptions.rst

3756

(I'd avoid the word For, because of keyword for.)
A requires-expression is the whole expression, requires + param-list + body: https://eel.is/c++draft/expr.prim.req#nt:requires-expression
A requires-clause is the whole clause, requires + logical-or-expr: https://eel.is/c++draft/temp.pre#nt:requires-clause
Does that resolve your concern about the word In?

[HazardyKnusperkeks]

• Rebased
• Updated
• Tests changed

[HazardyKnusperkeks]

clang/docs/ClangFormatStyleOptions.rst

3756

My concern is: In may refer to anywhere in the clause, but we are talking about one very specific point per clause. (Analogues for expression.)

The current version is very explicit, and I don't have a problem with such a long name.

clang/include/clang/Format/Format.h

3371

Non native here, but I think one could argue both were okay. I just copied what the other options state.

[• Quuxplusone]

clang/include/clang/Format/Format.h

3371

It's definitely not grammatical to talk about "opening parentheses, if there is one". But if this is just copying pre-existing wording, then OK, no objection to someone handling the grammar in a followup PR.

[HazardyKnusperkeks]

A friendly ping.

[HazardyKnusperkeks]

I need a response here. :) @Quuxplusone are the names okay for you?

[curdeius]

LGTM. My naming suggestion is not binding. I have no strong opinion on this, but a shorter name would get my 👍 :).
So please sync with other reviewers.

clang/docs/ClangFormatStyleOptions.rst

3755

Wouldn't AfterRequiresInClause/AfterRequiresInExpression be meaningful enough?

[• Quuxplusone]

clang/docs/ClangFormatStyleOptions.rst

3755

+1, I have no problem with AfterRequiresInClause and AfterRequiresInExpression!

[HazardyKnusperkeks]

clang/docs/ClangFormatStyleOptions.rst

3755

I will change the names accordingly before the commit.

They are much better.

clang/docs/ClangFormatStyleOptions.rst

**SpaceBeforeParensOptions** (``SpaceBeforeParensCustom``) :versionbadge:`clang-format 14`
   * ``bool AfterIfMacros`` If ``true``, put space between if macros and opening parentheses.
 
     .. code-block:: c++
 
        true:                                  false:
        IF (...)                        vs.    IF(...)
          <conditional-body>                     <conditional-body>
 
+  * ``bool AfterRequiresClause`` If ``true``, put space between requires keyword in a requires clause and
+    opening parentheses, if there is one.
+
+    .. code-block:: c++
+
+       true:                                  false:
+       template<typename T>            vs.    template<typename T>
+       requires (A<T> && B<T>)                requires(A<T> && B<T>)
+       ...                                    ...
+
+  * ``bool AfterRequiresExpression`` If ``true``, put space between requires keyword in a requires expression
+    and opening parentheses.
+
+    .. code-block:: c++
+
+       true:                                  false:
+       template<typename T>            vs.    template<typename T>
+       requires (T t) {                       requires(T t) {
+         ...                                    ...
+       }                                      }
+
   * ``bool BeforeNonEmptyParentheses`` If ``true``, put a space before opening parentheses only if the
     parentheses are not empty.
 
     .. code-block:: c++
 
        true:                                  false:
        void f (int a);                 vs.    void f();
        f (a);                                 f();

clang/include/clang/Format/Format.h

struct SpaceBeforeParensCustom {
     bool AfterFunctionDefinitionName;
     /// If ``true``, put space between if macros and opening parentheses.
     /// \code
     ///    true:                                  false:
     ///    IF (...)                        vs.    IF(...)
     ///      <conditional-body>                     <conditional-body>
     /// \endcode
     bool AfterIfMacros;
+    /// If ``true``, put space between requires keyword in a requires clause and
+    /// opening parentheses, if there is one.
+    /// \code
+    ///    true:                                  false:
+    ///    template<typename T>            vs.    template<typename T>
+    ///    requires (A<T> && B<T>)                requires(A<T> && B<T>)
+    ///    ...                                    ...
+    /// \endcode
+    bool AfterRequiresClause;
+    /// If ``true``, put space between requires keyword in a requires expression
+    /// and opening parentheses.
+    /// \code
+    ///    true:                                  false:
+    ///    template<typename T>            vs.    template<typename T>
+    ///    requires (T t) {                       requires(T t) {
+    ///      ...                                    ...
+    ///    }                                      }
+    /// \endcode
+    bool AfterRequiresExpression;
     /// If ``true``, put a space before opening parentheses only if the
     /// parentheses are not empty.
     /// \code
     ///    true:                                  false:
     ///    void f (int a);                 vs.    void f();
     ///    f (a);                                 f();
     /// \endcode
     bool BeforeNonEmptyParentheses;
 
     SpaceBeforeParensCustom()
         : AfterControlStatements(false), AfterForeachMacros(false),
           AfterFunctionDeclarationName(false),
           AfterFunctionDefinitionName(false), AfterIfMacros(false),
+          AfterRequiresClause(false), AfterRequiresExpression(false),
           BeforeNonEmptyParentheses(false) {}
 
     bool operator==(const SpaceBeforeParensCustom &Other) const {
       return AfterControlStatements == Other.AfterControlStatements &&
              AfterForeachMacros == Other.AfterForeachMacros &&
              AfterFunctionDeclarationName ==
                  Other.AfterFunctionDeclarationName &&
              AfterFunctionDefinitionName == Other.AfterFunctionDefinitionName &&
              AfterIfMacros == Other.AfterIfMacros &&
+             AfterRequiresClause == Other.AfterRequiresClause &&
+             AfterRequiresExpression == Other.AfterRequiresExpression &&
              BeforeNonEmptyParentheses == Other.BeforeNonEmptyParentheses;
     }
   };
 
   /// Control of individual space before parentheses.
   ///
   /// If ``SpaceBeforeParens`` is set to ``Custom``, use this to specify
   /// how each individual space before parentheses case should be handled.

clang/lib/Format/Format.cpp

template <> struct MappingTraits<FormatStyle::SpaceBeforeParensCustom> {
   static void mapping(IO &IO, FormatStyle::SpaceBeforeParensCustom &Spacing) {
     IO.mapOptional("AfterControlStatements", Spacing.AfterControlStatements);
     IO.mapOptional("AfterForeachMacros", Spacing.AfterForeachMacros);
     IO.mapOptional("AfterFunctionDefinitionName",
                    Spacing.AfterFunctionDefinitionName);
     IO.mapOptional("AfterFunctionDeclarationName",
                    Spacing.AfterFunctionDeclarationName);
     IO.mapOptional("AfterIfMacros", Spacing.AfterIfMacros);
+    IO.mapOptional("AfterRequiresClause", Spacing.AfterRequiresClause);
+    IO.mapOptional("AfterRequiresExpression", Spacing.AfterRequiresExpression);
     IO.mapOptional("BeforeNonEmptyParentheses",
                    Spacing.BeforeNonEmptyParentheses);
   }
 };
 
 template <> struct MappingTraits<FormatStyle::RawStringFormat> {
   static void mapping(IO &IO, FormatStyle::RawStringFormat &Format) {
     IO.mapOptional("Language", Format.Language);
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.SpaceAfterCStyleCast = false;
   LLVMStyle.SpaceAfterLogicalNot = false;
   LLVMStyle.SpaceAfterTemplateKeyword = true;
   LLVMStyle.SpaceAroundPointerQualifiers = FormatStyle::SAPQ_Default;
   LLVMStyle.SpaceBeforeCaseColon = false;
   LLVMStyle.SpaceBeforeCtorInitializerColon = true;
   LLVMStyle.SpaceBeforeInheritanceColon = true;
   LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
+  LLVMStyle.SpaceBeforeParensOptions = {};
   LLVMStyle.SpaceBeforeParensOptions.AfterControlStatements = true;
   LLVMStyle.SpaceBeforeParensOptions.AfterForeachMacros = true;
   LLVMStyle.SpaceBeforeParensOptions.AfterIfMacros = true;
   LLVMStyle.SpaceBeforeRangeBasedForLoopColon = true;
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.SpaceBeforeCpp11BracedList = false;
   LLVMStyle.SpaceBeforeSquareBrackets = false;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;

clang/lib/Format/TokenAnnotator.cpp

bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
   // Space between template and attribute.
   // e.g. template <typename T> [[nodiscard]] ...
   if (Left.is(TT_TemplateCloser) && Right.is(TT_AttributeSquare))
     return true;
   // Space before parentheses common for all languages
   if (Right.is(tok::l_paren)) {
     if (Left.is(TT_TemplateCloser) && Right.isNot(TT_FunctionTypeLParen))
       return spaceRequiredBeforeParens(Right);
-    if (Left.is(tok::kw_requires))
-      return spaceRequiredBeforeParens(Right);
+    if (Left.is(tok::kw_requires)) {
+      if (spaceRequiredBeforeParens(Right))
+        return true;
+      if (Left.is(TT_RequiresClause))
+        return Style.SpaceBeforeParensOptions.AfterRequiresClause;
+      else {
+        assert(Left.is(TT_RequiresExpression));
+        return Style.SpaceBeforeParensOptions.AfterRequiresExpression;
+      }
+    }
     if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
         (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
       return true;
     if (Left.is(TT_ForEachMacro))
       return (Style.SpaceBeforeParensOptions.AfterForeachMacros ||
               spaceRequiredBeforeParens(Right));
     if (Left.is(TT_IfMacro))
       return (Style.SpaceBeforeParensOptions.AfterIfMacros ||

clang/lib/Format/UnwrappedLineParser.cpp

void UnwrappedLineParser::parseRequiresClauseOrExpression(
   } else {
     parseConstraintExpression(OriginalLevel);
   }
 }
 
 void UnwrappedLineParser::parseConstraintExpression(
     unsigned int OriginalLevel) {
   // requires Id<T> && Id<T> || Id<T>
+
+  if (FormatTok->Tok.is(tok::kw_requires)) {
+    assert(FormatTok->Previous);
+    assert(FormatTok->Previous->is(TT_RequiresClause));
+    FormatTok->setType(TT_RequiresExpression);
+  }
+
   while (
       FormatTok->isOneOf(tok::identifier, tok::kw_requires, tok::coloncolon)) {
     nextToken();
     while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::less,
                               tok::greater, tok::comma, tok::ellipsis)) {
       if (FormatTok->Tok.is(tok::less)) {
         parseBracedList(/*ContinueOnSemicolons=*/false, /*IsEnum=*/false,
                         /*ClosingBraceKind=*/tok::greater);

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, ConfigurableSpaceBeforeParens) {
   verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");",
                SomeSpace2);
   verifyFormat("int f() throw (Deprecated);", SomeSpace2);
   verifyFormat("typedef void (*cb) (int);", SomeSpace2);
   verifyFormat("T A::operator()();", SomeSpace2);
   verifyFormat("X A::operator++ (T);", SomeSpace2);
   verifyFormat("int x = int (y);", SomeSpace2);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace2);
+
+  auto SpaceAfterRequires = getLLVMStyle();
+  SpaceAfterRequires.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  verifyFormat(
+      "template <typename T> void func(T) requires(trait<T> && trait<T>) {\n"
+      "  typename T::size_type;\n"
+      "  { t.size() } -> std::same_as<typename T::size_type>;\n"
+      "}\n"
+      "{}",
+      SpaceAfterRequires);
+  verifyFormat("template <typename T> void func(T) requires requires(T &&t) {\n"
+               "  typename T::size_type;\n"
+               "  { t.size() } -> std::same_as<typename T::size_type>;\n"
+               "}\n"
+               "{}",
+               SpaceAfterRequires);
+
+  SpaceAfterRequires.SpaceBeforeParensOptions.AfterRequiresClause = true;
+  SpaceAfterRequires.SpaceBeforeParensOptions.AfterRequiresExpression = false;
+  verifyFormat(
+      "template <typename T> void func(T) requires (trait<T> && trait<T>) {\n"
+      "  typename T::size_type;\n"
+      "  { t.size() } -> std::same_as<typename T::size_type>;\n"
+      "}\n"
+      "{}",
+      SpaceAfterRequires);
+  verifyFormat(
+      "template <typename T> void func(T) requires requires(T &&t) {\n"
+      "  typename T::size_type;\n"
+      "  { t.size() } -> std::same_as<typename T::size_type>;\n"
+      "}\n"
+      "{}",
+      SpaceAfterRequires);
+
+  SpaceAfterRequires.SpaceBeforeParensOptions.AfterRequiresClause = false;
+  SpaceAfterRequires.SpaceBeforeParensOptions.AfterRequiresExpression = true;
+  verifyFormat(
+      "template <typename T> void func(T) requires(trait<T> && trait<T>) {\n"
+      "  typename T::size_type;\n"
+      "  { t.size() } -> std::same_as<typename T::size_type>;\n"
+      "}\n"
+      "{}",
+      SpaceAfterRequires);
+  verifyFormat(
+      "template <typename T> void func(T) requires requires (T &&t) {\n"
+      "  typename T::size_type;\n"
+      "  { t.size() } -> std::same_as<typename T::size_type>;\n"
+      "}\n"
+      "{}",
+      SpaceAfterRequires);
+
+  SpaceAfterRequires.SpaceBeforeParensOptions.AfterRequiresClause = true;
+  SpaceAfterRequires.SpaceBeforeParensOptions.AfterRequiresExpression = true;
+  verifyFormat(
+      "template <typename T> void func(T) requires (trait<T> && trait<T>) {\n"
+      "  typename T::size_type;\n"
+      "  { t.size() } -> std::same_as<typename T::size_type>;\n"
+      "}\n"
+      "{}",
+      SpaceAfterRequires);
+  verifyFormat(
+      "template <typename T> void func(T) requires requires (T &&t) {\n"
+      "  typename T::size_type;\n"
+      "  { t.size() } -> std::same_as<typename T::size_type>;\n"
+      "}\n"
+      "{}",
+      SpaceAfterRequires);
 }
 
 TEST_F(FormatTest, SpaceAfterLogicalNot) {
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpaceAfterLogicalNot = true;
 
   verifyFormat("bool x = ! y", Spaces);
   verifyFormat("if (! isFailure())", Spaces);
verifyFormat(
       "void func(T) requires(std::is_integral_v<T> && std::is_signed_v<T>) {}",
       Style);
   verifyFormat("template <typename T> void func(T) requires requires(T &&t) {\n"
                "  typename T::size_type;\n"
                "  { t.size() } -> std::same_as<typename T::size_type>;\n"
                "}\n"
                "{}",
                Style);
+
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  Style.SpaceBeforeParensOptions.AfterRequiresClause = true;
+  Style.SpaceBeforeParensOptions.AfterRequiresExpression = true;
+  verifyFormat("template <typename T>\n"
+               "  requires (std::is_integral_v<T> && std::is_signed_v<T>)\n"
+               "void func(T) {}",
+               Style);
+  verifyFormat(
+      "template <typename T> void func(T) requires requires (T &&t) {\n"
+      "  typename T::size_type;\n"
+      "  { t.size() } -> std::same_as<typename T::size_type>;\n"
+      "}\n"
+      "{}",
+      Style);
 }
 
 TEST_F(FormatTest, StatementAttributeLikeMacros) {
   FormatStyle Style = getLLVMStyle();
   StringRef Source = "void Foo::slot() {\n"
                      "  unsigned char MyChar = 'x';\n"
                      "  emit signal(MyChar);\n"
                      "  Q_EMIT signal(MyChar);\n"