https://github.com/ameerj created https://github.com/llvm/llvm-project/pull/95013
By default, clang-format packs binary operations, but it may be desirable to have compound operations be on individual lines instead of being packed. This PR adds the option `BinPackBinaryOperations`, which mimics the behavior of `BinPackArguments` and `BinPackParameters`, but applied to binary operations. When `BinPackBinaryOperations` is set to `false`, binary operations will either be all on the same line, or each operation will have one line each. This applies to all logical and arithmetic/bitwise binary operations Maybe partially addresses #79487 ? >From a50f3d4395efd09eea8ba2e750bb785857f9a550 Mon Sep 17 00:00:00 2001 From: ameerj <ameer...@outlook.com> Date: Mon, 10 Jun 2024 12:09:40 -0400 Subject: [PATCH 1/3] Add BinPackBinaryOperations --- clang/include/clang/Format/Format.h | 16 ++++++++++++++++ clang/lib/Format/ContinuationIndenter.cpp | 15 +++++++++++++++ clang/lib/Format/Format.cpp | 2 ++ 3 files changed, 33 insertions(+) diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 4fd6e013df25b..f30d767600061 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -1192,6 +1192,21 @@ struct FormatStyle { /// \version 3.7 bool BinPackArguments; + /// If ``false``, binary operations will either be all on the same line + /// or each operation will have one line each. + /// \code + /// true: + /// aaaaaaaaaaaaaaaaaaaa && aaaaaaaaaaaaaaaaaaaa || + /// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa; + /// + /// false: + /// aaaaaaaaaaaaaaaaaaaa && + /// aaaaaaaaaaaaaaaaaaaa || + /// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa; + /// \endcode + /// \version 19 + bool BinPackBinaryOperations; + /// If ``false``, a function declaration's or function definition's /// parameters will either all be on the same line or will have one line each. /// \code @@ -4978,6 +4993,7 @@ struct FormatStyle { R.AlwaysBreakBeforeMultilineStrings && AttributeMacros == R.AttributeMacros && BinPackArguments == R.BinPackArguments && + BinPackBinaryOperations == R.BinPackBinaryOperations && BinPackParameters == R.BinPackParameters && BitFieldColonSpacing == R.BitFieldColonSpacing && BracedInitializerIndentWidth == R.BracedInitializerIndentWidth && diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index b07360425ca6e..0780d219b68b4 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -146,6 +146,14 @@ static bool startsNextParameter(const FormatToken &Current, Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma)); } +// Returns \c true if \c Current starts a new operand in a binary operation. +static bool startsNextOperand(const FormatToken &Current, + const FormatStyle &Style) { + const FormatToken &Previous = *Current.Previous; + return Previous.is(TT_BinaryOperator) && !Current.isTrailingComment() && + (Previous.getPrecedence() > prec::Conditional); +} + static bool opensProtoMessageField(const FormatToken &LessTok, const FormatStyle &Style) { if (LessTok.isNot(tok::less)) @@ -837,6 +845,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, } if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style)) CurrentState.NoLineBreak = true; + if (!Style.BinPackBinaryOperations && startsNextOperand(Current, Style)) + CurrentState.NoLineBreak = true; + if (startsSegmentOfBuilderTypeCall(Current) && State.Column > getNewLineColumn(State)) { CurrentState.ContainsUnwrappedBuilder = true; @@ -1203,6 +1214,10 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State, } } + if (!Style.BinPackBinaryOperations && Previous.is(TT_BinaryOperator) && + (Previous.getPrecedence() > prec::Conditional)) { + CurrentState.BreakBeforeParameter = true; + } return Penalty; } diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index c015e03fa15e7..4d71ce95afe1d 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -935,6 +935,7 @@ template <> struct MappingTraits<FormatStyle> { Style.AlwaysBreakBeforeMultilineStrings); IO.mapOptional("AttributeMacros", Style.AttributeMacros); IO.mapOptional("BinPackArguments", Style.BinPackArguments); + IO.mapOptional("BinPackBinaryOperations", Style.BinPackBinaryOperations); IO.mapOptional("BinPackParameters", Style.BinPackParameters); IO.mapOptional("BitFieldColonSpacing", Style.BitFieldColonSpacing); IO.mapOptional("BracedInitializerIndentWidth", @@ -1439,6 +1440,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.AlwaysBreakBeforeMultilineStrings = false; LLVMStyle.AttributeMacros.push_back("__capability"); LLVMStyle.BinPackArguments = true; + LLVMStyle.BinPackBinaryOperations = true; LLVMStyle.BinPackParameters = true; LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both; LLVMStyle.BracedInitializerIndentWidth = std::nullopt; >From 1fe691c6ee0393cc3f40f0c5e94e01fe79a909c3 Mon Sep 17 00:00:00 2001 From: ameerj <ameer...@outlook.com> Date: Mon, 10 Jun 2024 12:43:56 -0400 Subject: [PATCH 2/3] Add BinPackBinaryOperations unit tests --- clang/unittests/Format/FormatTest.cpp | 80 +++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index fb57333858529..cb20c5db4884b 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -27492,6 +27492,86 @@ TEST_F(FormatTest, SpaceBetweenKeywordAndLiteral) { verifyFormat("return sizeof \"5\";"); } +TEST_F(FormatTest, BinPackBinaryOperations) { + auto Style = getLLVMStyle(); + Style.BinPackBinaryOperations = false; + + // Logical operations + verifyFormat("if (condition1 && condition2) {\n" + "}", + Style); + verifyFormat("if (condition1 && // comment\n" + " condition2 &&\n" + " (condition3 || condition4) && // comment\n" + " condition5 &&\n" + " condition6) {\n" + "}", + Style); + verifyFormat("if (loooooooooooooooooooooongcondition1 &&\n" + " loooooooooooooooooooooongcondition2) {\n" + "}", + Style); + + // Arithmetic + verifyFormat("const int result = lhs + rhs;\n", Style); + verifyFormat("const int result = loooooooooooooooooooooongop1 +\n" + " loooooooooooooooooooooongop2 +\n" + " loooooooooooooooooooooongop3;\n", + Style); + + // clang-format off + verifyFormat("const int result =\n" + " operand1 + operand2 - (operand3 + operand4) - operand5 * operand6;\n", + Style); + // clang-format on + + verifyFormat("const int result = longOperand1 +\n" + " longOperand2 -\n" + " (longOperand3 + longOperand4) -\n" + " longOperand5 * longOperand6;\n", + Style); + + Style.BinPackBinaryOperations = true; + + // Logical operations + verifyFormat("if (condition1 && condition2) {\n" + "}", + Style); + + // clang-format off + verifyFormat("if (condition1 && condition2 && (condition3 || condition4) && condition5 &&\n" + " condition6) {\n" + "}", + Style); + // clang-format on + + verifyFormat("if (loooooooooooooooooooooongcondition1 &&\n" + " loooooooooooooooooooooongcondition2) {\n" + "}", + Style); + + // Arithmetic + verifyFormat("const int result = lhs + rhs;\n", Style); + + // clang-format off + verifyFormat("const int result = loooooooooooooooooooooongop1 + loooooooooooooooooooooongop2 +\n" + " loooooooooooooooooooooongop3;\n", + Style); + // clang-format on + + // clang-format off + verifyFormat("const int result = longOperand1 + longOperand2 - (longOperand3 + longOperand4) -\n" + " longOperand5 * longOperand6;\n", + Style); + // clang-format on + + // clang-format off + verifyFormat("const int result =\n" + " operand1 + operand2 - (operand3 + operand4) - operand5 * operand6;\n", + Style); + // clang-format on +} + } // namespace } // namespace test } // namespace format >From d95bf096387aa1812b83604e05edd22cc22d9584 Mon Sep 17 00:00:00 2001 From: ameerj <ameer...@outlook.com> Date: Mon, 10 Jun 2024 13:18:07 -0400 Subject: [PATCH 3/3] Update ClangFormatStyleOptions.rst --- clang/docs/ClangFormatStyleOptions.rst | 17 +++++++++++++++++ clang/include/clang/Format/Format.h | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index bb00c20922d36..246d98eef63e8 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -2065,6 +2065,23 @@ the configuration (without a prefix: ``Auto``). aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa); } +.. _BinPackBinaryOperations: + +**BinPackBinaryOperations** (``Boolean``) :versionbadge:`clang-format 19` :ref:`¶ <BinPackBinaryOperations>` + If ``false``, binary operations will either be all on the same line + or each operation will have one line each. + + .. code-block:: c++ + + true: + aaaaaaaaaaaaaaaaaaaa && aaaaaaaaaaaaaaaaaaaa || + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa; + + false: + aaaaaaaaaaaaaaaaaaaa && + aaaaaaaaaaaaaaaaaaaa || + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa; + .. _BinPackParameters: **BinPackParameters** (``Boolean``) :versionbadge:`clang-format 3.7` :ref:`¶ <BinPackParameters>` diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index f30d767600061..19b9c60ab6e99 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -1197,7 +1197,7 @@ struct FormatStyle { /// \code /// true: /// aaaaaaaaaaaaaaaaaaaa && aaaaaaaaaaaaaaaaaaaa || - /// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa; + /// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa; /// /// false: /// aaaaaaaaaaaaaaaaaaaa && _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits