[clang-format] AllowShortCompoundRequirementOnASingleLine

[Backl1ght]

Summary

clang-format brace wrapping did not take requires into consideration, compound requirements will be affected by BraceWrapping.AfterFunction.

I plan to add 3 options AllowShortRequiresExpressionOnASingleLine, AllowShortCompoundRequirementOnASingleLine and BraceWrapping.AfterRequiresExpression.

This patch is for AllowShortCompoundRequirementOnASingleLine.

https://github.com/llvm/llvm-project/issues/59412

[HazardyKnusperkeks]

See the comments on D139786. And you can base your commits on each other, and then add a child rsp. parent revision here in phabricator, that will prevent merge conflicts.

[MyDeveloperDay]

clang/lib/Format/Format.cpp

941

haven't we use "Requires" in other options? What is the definition of Compound?

I might be tempted for this to be AllShortRequiresOnASingleLine but then it be an enum with the following options

Leave
Never
Always
Compound

1446

why would the default be true, is that what happens today?

[Backl1ght]

clang/lib/Format/Format.cpp

941

There are some options related to requires like IndentRequiresClause, RequiresClausePosition, etc. But as for single line and brace wrapping, requires is not yet considered.

"Compound" is from Compound Requirements section in this page https://en.cppreference.com/w/cpp/language/requires

I think this is a better way, I would try implementing it.

1446

yes

[MyDeveloperDay]

clang/lib/Format/Format.cpp

1446

just to clarify so I'm sure (without me having to try it myself), if you hadn't introduce this option it would be the equivalent of true.

The only reason I say is we get complained at when we change the default from not doing something to doing something. Even if that means before we left it alone.

So mostly we normally find the options go through an evolution from bool->enum->struct, sometimes it can be better to introduce an enum so we can have "Leave" as the default

in such circumstances you let the old behaviour be the default, that way we know. That previously unformatted compound statements won't be touch in any way.

else if (Style.AllowShortCompoundRequirementOnASingleLine != Leave  && .......)
{

Users then have to "positively" buy into your style change one way or another. Rather than us imposing a default even if that default seems perfectly reasonable.

[HazardyKnusperkeks]

clang/lib/Format/Format.cpp

1446

The true as default is correct.

[MyDeveloperDay]

clang/lib/Format/Format.cpp

1446

Then if @HazardyKnusperkeks you think this is ok, I'm ok with it too.

[rymiel]

clang/lib/Format/Format.cpp

941

A compound-requirement is what the standard calls it, so I think the name is fine.

I'm not sure if collating it with other options as suggested is a good idea, since a requires-expression can contain multiple compound-requirements, such as:

requires {
  { E1 } -> C;
  { E2 } -> D<A1>;
};

This requires-expression has two compound-requirements, and two formatting of those two individually is what's being controlled by the option.

Although I will agree it should probably still be an enum, but in that case, it probably shouldn't start with "Allow"

[rymiel]

I see nothing wrong with this patch alone as it currently stands, since it's a quite simple change to the LineJoiner, and I see it as one of the final stopgaps in clang-format's support for requires clauses, and I think it definitely beats the current buggy behavior where behaviour is conflated with BraceWrapping.AfterFunction.
But I'd like other reviewers to maybe have another look?

clang/docs/ClangFormatStyleOptions.rst

.. code-block:: c++
     switch (a) {                    vs.     switch (a) {
     case 1: x = 1; break;                   case 1:
     case 2: return;                           x = 1;
     }                                         break;
                                             case 2:
                                               return;
                                             }
 
+.. _AllowShortCompoundRequirementOnASingleLine:
+
+**AllowShortCompoundRequirementOnASingleLine** (``Boolean``) :versionbadge:`clang-format 16` :ref:`¶ <AllowShortCompoundRequirementOnASingleLine>`
+  Allow short compound requirement on a single line.
+
+  .. code-block:: c++
+
+    true:
+    template <typename T>
+    concept c = requires(T x) {
+      { x + 1 } -> std::same_as<int>;
+    };
+
+    false:
+    template <typename T>
+    concept c = requires(T x) {
+      {
+        x + 1
+      } -> std::same_as<int>;
+    };
+
 .. _AllowShortEnumsOnASingleLine:
 
 **AllowShortEnumsOnASingleLine** (``Boolean``) :versionbadge:`clang-format 11` :ref:`¶ <AllowShortEnumsOnASingleLine>`
   Allow short enums on a single line.
 
   .. code-block:: c++
 
     true:

clang/include/clang/Format/Format.h

struct FormatStyle {
   ///   }                                         break;
   ///                                           case 2:
   ///                                             return;
   ///                                           }
   /// \endcode
   /// \version 3.6
   bool AllowShortCaseLabelsOnASingleLine;
 
+  /// Allow short compound requirement on a single line.
+  /// \code
+  ///   true:
+  ///   template <typename T>
+  ///   concept c = requires(T x) {
+  ///     { x + 1 } -> std::same_as<int>;
+  ///   };
+  ///
+  ///   false:
+  ///   template <typename T>
+  ///   concept c = requires(T x) {
+  ///     {
+  ///       x + 1
+  ///     } -> std::same_as<int>;
+  ///   };
+  /// \endcode
+  /// \version 16
+  bool AllowShortCompoundRequirementOnASingleLine;
+
   /// Allow short enums on a single line.
   /// \code
   ///   true:
   ///   enum { A, B } myEnum;
   ///
   ///   false:
   ///   enum {
   ///     A,
return AccessModifierOffset == R.AccessModifierOffset &&
            AllowAllArgumentsOnNextLine == R.AllowAllArgumentsOnNextLine &&
            AllowAllParametersOfDeclarationOnNextLine ==
                R.AllowAllParametersOfDeclarationOnNextLine &&
            AllowBreakBeforeNoexceptSpecifier ==
                R.AllowBreakBeforeNoexceptSpecifier &&
            AllowShortBlocksOnASingleLine == R.AllowShortBlocksOnASingleLine &&
            AllowShortCaseLabelsOnASingleLine ==
                R.AllowShortCaseLabelsOnASingleLine &&
+           AllowShortCompoundRequirementOnASingleLine ==
+               R.AllowShortCompoundRequirementOnASingleLine &&
            AllowShortEnumsOnASingleLine == R.AllowShortEnumsOnASingleLine &&
            AllowShortFunctionsOnASingleLine ==
                R.AllowShortFunctionsOnASingleLine &&
            AllowShortIfStatementsOnASingleLine ==
                R.AllowShortIfStatementsOnASingleLine &&
            AllowShortLambdasOnASingleLine == R.AllowShortLambdasOnASingleLine &&
            AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&
            AlwaysBreakAfterReturnType == R.AlwaysBreakAfterReturnType &&

clang/lib/Format/Format.cpp

static void mapping(IO &IO, FormatStyle &Style) {
     IO.mapOptional("AllowAllParametersOfDeclarationOnNextLine",
                    Style.AllowAllParametersOfDeclarationOnNextLine);
     IO.mapOptional("AllowBreakBeforeNoexceptSpecifier",
                    Style.AllowBreakBeforeNoexceptSpecifier);
     IO.mapOptional("AllowShortBlocksOnASingleLine",
                    Style.AllowShortBlocksOnASingleLine);
     IO.mapOptional("AllowShortCaseLabelsOnASingleLine",
                    Style.AllowShortCaseLabelsOnASingleLine);
+    IO.mapOptional("AllowShortCompoundRequirementOnASingleLine",
+                   Style.AllowShortCompoundRequirementOnASingleLine);
     IO.mapOptional("AllowShortEnumsOnASingleLine",
                    Style.AllowShortEnumsOnASingleLine);
     IO.mapOptional("AllowShortFunctionsOnASingleLine",
                    Style.AllowShortFunctionsOnASingleLine);
     IO.mapOptional("AllowShortIfStatementsOnASingleLine",
                    Style.AllowShortIfStatementsOnASingleLine);
     IO.mapOptional("AllowShortLambdasOnASingleLine",
                    Style.AllowShortLambdasOnASingleLine);
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.AlignConsecutiveShortCaseStatements = {};
   LLVMStyle.AlignTrailingComments = {};
   LLVMStyle.AlignTrailingComments.Kind = FormatStyle::TCAS_Always;
   LLVMStyle.AlignTrailingComments.OverEmptyLines = 0;
   LLVMStyle.AllowAllArgumentsOnNextLine = true;
   LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;
   LLVMStyle.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
   LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
+  LLVMStyle.AllowShortCompoundRequirementOnASingleLine = true;
   LLVMStyle.AllowShortEnumsOnASingleLine = true;
   LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
   LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
   LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;
   LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;

clang/lib/Format/UnwrappedLineFormatter.cpp

if (PreviousLine && Style.BraceWrapping.SplitEmptyRecord &&
       }
     }
 
     if (TheLine->Last->is(tok::l_brace)) {
       bool ShouldMerge = false;
       // Try to merge records.
       if (TheLine->Last->is(TT_EnumLBrace)) {
         ShouldMerge = Style.AllowShortEnumsOnASingleLine;
+      } else if (TheLine->Last->is(TT_RequiresExpressionLBrace)) {
+        ShouldMerge = Style.AllowShortCompoundRequirementOnASingleLine;
       } else if (TheLine->Last->isOneOf(TT_ClassLBrace, TT_StructLBrace)) {
         // NOTE: We use AfterClass (whereas AfterStruct exists) for both classes
         // and structs, but it seems that wrapping is still handled correctly
         // elsewhere.
         ShouldMerge = !Style.BraceWrapping.AfterClass ||
                       (NextLine.First->is(tok::r_brace) &&
                        !Style.BraceWrapping.SplitEmptyRecord);
       } else if (TheLine->InPPDirective ||

clang/unittests/Format/ConfigParseTest.cpp

#define CHECK_PARSE_NESTED_VALUE(TEXT, STRUCT, FIELD, VALUE)                   \
   EXPECT_EQ(VALUE, Style.STRUCT.FIELD) << "Unexpected value after parsing!"
 
 TEST(ConfigParseTest, ParsesConfigurationBools) {
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
+  CHECK_PARSE_BOOL(AllowShortCompoundRequirementOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortEnumsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakStringLiterals);
   CHECK_PARSE_BOOL(CompactNamespaces);

clang/unittests/Format/FormatTest.cpp

verifyFormat("typedef enum\n"
                "{\n"
                "  A,\n"
                "  B,\n"
                "  C\n"
                "} ShortEnum1, ShortEnum2;",
                Style);
 }
 
+TEST_F(FormatTest, ShortCompoundRequirement) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_TRUE(Style.AllowShortCompoundRequirementOnASingleLine);
+  verifyFormat("template <typename T>\n"
+               "concept c = requires(T x) {\n"
+               "  { x + 1 } -> std::same_as<int>;\n"
+               "};",
+               Style);
+  verifyFormat("template <typename T>\n"
+               "concept c = requires(T x) {\n"
+               "  { x + 1 } -> std::same_as<int>;\n"
+               "  { x + 2 } -> std::same_as<int>;\n"
+               "};",
+               Style);
+  Style.AllowShortCompoundRequirementOnASingleLine = false;
+  verifyFormat("template <typename T>\n"
+               "concept c = requires(T x) {\n"
+               "  {\n"
+               "    x + 1\n"
+               "  } -> std::same_as<int>;\n"
+               "};",
+               Style);
+  verifyFormat("template <typename T>\n"
+               "concept c = requires(T x) {\n"
+               "  {\n"
+               "    x + 1\n"
+               "  } -> std::same_as<int>;\n"
+               "  {\n"
+               "    x + 2\n"
+               "  } -> std::same_as<int>;\n"
+               "};",
+               Style);
+}
+
 TEST_F(FormatTest, ShortCaseLabels) {
   FormatStyle Style = getLLVMStyle();
   Style.AllowShortCaseLabelsOnASingleLine = true;
   verifyFormat("switch (a) {\n"
                "case 1: x = 1; break;\n"
                "case 2: return;\n"
                "case 3:\n"
                "case 4:\n"