[clang-format] AllowShortRequiresExpressionOnASingleLine

[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 AllowShortRequiresExpressionOnASingleLine.

[rymiel]

Please re-upload with full context. See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

clang/lib/Format/Format.cpp

811

I'm pretty sure these options should be in alphabetical order (and below too)

[HazardyKnusperkeks]

I don't know if adding two options at once (and I think you want to add a third), splitting that up might be a better idea.

Anyway you need to tun clang/docs/tools/dump_format_style.py and add an entry in the changelog.

clang/include/clang/Format/Format.h

542

We have both, but I like the variant with true and false next to each other, not below.

clang/lib/Format/UnwrappedLineParser.cpp

1105

Unrelated.

clang/unittests/Format/FormatTest.cpp

2911

You need to add the Style.

[Backl1ght]

I just find out I miss one case like below

template<typename T>
concept c = requires(T x) { { x + 1 } -> std::same_as<int>; };

Since I split AllowShortRequiresExpressionOnASingleLine and AllowShortCompoundRequirementOnASingleLine into two pathes, I can not fix this until AllowShortCompoundRequirementOnASingleLine got merged.

[HazardyKnusperkeks]

Please mark comments as done.

clang/lib/Format/Format.cpp

1270

Please initialize to false, because that is what we are doing right now.

clang/unittests/Format/FormatTest.cpp

2904

Please add a test where the clause is too long and will still be wrapped. Reduce the column limit and use some longer identifiers.

[MyDeveloperDay]

ditto my concerns with D139834: [clang-format] AllowShortCompoundRequirementOnASingleLine as to if we should have a Leave option

clang/include/clang/Format/Format.h

   /// \version 11
   bool AllowShortEnumsOnASingleLine;
 
+  /// Allow short requires expression on a single line.
+  /// \code
+  ///   true:
+  ///   template <typename T>
+  ///   concept c = requires(T a, T b) { a + b; };
+  ///
+  ///   false:
+  ///   template <typename T>
+  ///   concept c = requires(T a, T b) {
+  ///     a + b;
+  ///   };
+  /// \endcode
+  /// \version 16
+  bool AllowShortRequiresExpressionOnASingleLine;
+
+  /// 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;
+
   /// Different styles for merging short functions containing at most one
   /// statement.
   enum ShortFunctionStyle : int8_t {

            AllowShortCaseLabelsOnASingleLine ==
                R.AllowShortCaseLabelsOnASingleLine &&
            AllowShortEnumsOnASingleLine == R.AllowShortEnumsOnASingleLine &&
+           AllowShortRequiresExpressionOnASingleLine ==
+               R.AllowShortRequiresExpressionOnASingleLine &&
+           AllowShortCompoundRequirementOnASingleLine ==
+               R.AllowShortCompoundRequirementOnASingleLine &&
            AllowShortFunctionsOnASingleLine ==
                R.AllowShortFunctionsOnASingleLine &&
            AllowShortIfStatementsOnASingleLine ==

clang/lib/Format/Format.cpp

                    Style.AllowShortCaseLabelsOnASingleLine);
     IO.mapOptional("AllowShortEnumsOnASingleLine",
                    Style.AllowShortEnumsOnASingleLine);
+    IO.mapOptional("AllowShortRequiresExpressionOnASingleLine",
+                   Style.AllowShortRequiresExpressionOnASingleLine);
+    IO.mapOptional("AllowShortCompoundRequirementOnASingleLine",
+                   Style.AllowShortCompoundRequirementOnASingleLine);
     IO.mapOptional("AllowShortFunctionsOnASingleLine",
                    Style.AllowShortFunctionsOnASingleLine);
     IO.mapOptional("AllowShortIfStatementsOnASingleLine",

   LLVMStyle.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
   LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
   LLVMStyle.AllowShortEnumsOnASingleLine = true;
+  LLVMStyle.AllowShortRequiresExpressionOnASingleLine = true;
+  LLVMStyle.AllowShortCompoundRequirementOnASingleLine = true;
   LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
   LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
   LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;

clang/lib/Format/UnwrappedLineFormatter.cpp

       // Try to merge records.
       if (TheLine->Last->is(TT_EnumLBrace)) {
         ShouldMerge = Style.AllowShortEnumsOnASingleLine;
+      } else if (TheLine->Last->is(TT_RequiresExpressionLBrace)) {
+        ShouldMerge = Style.AllowShortRequiresExpressionOnASingleLine;
+      } else if (TheLine->Last->is(TT_CompoundRequirementLBrace)) {
+        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

clang/lib/Format/UnwrappedLineParser.cpp

   FormatTok->setBlockKind(BK_Block);
   const FormatToken *OpeningBrace = FormatTok;
   nextToken();
+
+  if (NextLBracesType == TT_CompoundRequirementLBrace &&
+      !Style.AllowShortRequiresExpressionOnASingleLine) {
+    addUnwrappedLine();
+  }
+
   {
     bool SkipIndent = (Style.isJavaScript() &&
                        (isGoogScope(*Line) || isIIFE(*Line, Keywords)));

     flushComments(isOnNewLine(*FormatTok));
     Line->Level -= SkipIndent ? 0 : 1;
   }
+
   nextToken();
 }
 

clang/unittests/Format/ConfigParseTest.cpp

   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortEnumsOnASingleLine);
+  CHECK_PARSE_BOOL(AllowShortRequiresExpressionOnASingleLine);
+  CHECK_PARSE_BOOL(AllowShortCompoundRequirementOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);

clang/unittests/Format/FormatTest.cpp

                Style);
 }
 
+TEST_F(FormatTest, ShortRequiresExpression) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_TRUE(Style.AllowShortRequiresExpressionOnASingleLine);
+  verifyFormat("template <typename T>\n"
+               "concept c = requires(T a, T b) { a + b; };");
+  Style.AllowShortRequiresExpressionOnASingleLine = false;
+  verifyFormat("template <typename T>\n"
+               "concept c = requires(T a, T b) {\n"
+               "  a + b;\n"
+               "};");
+  // TODO: support BraceWrapping.AfterRequiresExpression.
+}
+
+TEST_F(FormatTest, ShortCompoundRequirements) {
+  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.AllowShortCompoundRequirementOnASingleLine = false;
+  verifyFormat("template <typename T>\n"
+               "concept c = requires(T x) {\n"
+               "  {\n"
+               "    x + 1\n"
+               "  } -> std::same_as<int>;\n"
+               "};");
+}
+
 TEST_F(FormatTest, ShortCaseLabels) {
   FormatStyle Style = getLLVMStyle();
   Style.AllowShortCaseLabelsOnASingleLine = true;