Author: Ameer J Date: 2024-08-10T13:28:33-07:00 New Revision: c5a4291fb704b4cff469bab18ff7ebab41807564
URL: https://github.com/llvm/llvm-project/commit/c5a4291fb704b4cff469bab18ff7ebab41807564 DIFF: https://github.com/llvm/llvm-project/commit/c5a4291fb704b4cff469bab18ff7ebab41807564.diff LOG: [clang-format] Add BreakBinaryOperations configuration (#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 `BreakBinaryOperations` to break up large compound binary operations to be on one line each. This applies to all logical and arithmetic/bitwise binary operations Maybe partially addresses #79487 ? Closes #58014 Closes #57280 Added: Modified: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h clang/lib/Format/ContinuationIndenter.cpp clang/lib/Format/Format.cpp clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/ConfigParseTest.cpp clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index 29b08bca6a407f..72e1bd19b6b520 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -3300,6 +3300,46 @@ the configuration (without a prefix: ``Auto``). firstValue : SecondValueVeryVeryVeryVeryLong; +.. _BreakBinaryOperations: + +**BreakBinaryOperations** (``BreakBinaryOperationsStyle``) :versionbadge:`clang-format 20` :ref:`¶ <BreakBinaryOperations>` + The break constructor initializers style to use. + + Possible values: + + * ``BBO_Never`` (in configuration: ``Never``) + Don't break binary operations + + .. code-block:: c++ + + aaa + bbbb * ccccc - ddddd + + eeeeeeeeeeeeeeee; + + * ``BBO_OnePerLine`` (in configuration: ``OnePerLine``) + Binary operations will either be all on the same line, or each operation + will have one line each. + + .. code-block:: c++ + + aaa + + bbbb * + ccccc - + ddddd + + eeeeeeeeeeeeeeee; + + * ``BBO_RespectPrecedence`` (in configuration: ``RespectPrecedence``) + Binary operations of a particular precedence that exceed the column + limit will have one line each. + + .. code-block:: c++ + + aaa + + bbbb * ccccc - + ddddd + + eeeeeeeeeeeeeeee; + + + .. _BreakConstructorInitializers: **BreakConstructorInitializers** (``BreakConstructorInitializersStyle``) :versionbadge:`clang-format 5` :ref:`¶ <BreakConstructorInitializers>` diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 602f3edaf121cb..d002a9c747dd60 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -316,6 +316,8 @@ AST Matchers clang-format ------------ +- Adds ``BreakBinaryOperations`` option. + libclang -------- diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index fea5d2e17a0e28..ef6c76a070bfaa 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -2231,6 +2231,41 @@ struct FormatStyle { /// \version 3.7 bool BreakBeforeTernaryOperators; + /// Different ways to break binary operations. + enum BreakBinaryOperationsStyle : int8_t { + /// Don't break binary operations + /// \code + /// aaa + bbbb * ccccc - ddddd + + /// eeeeeeeeeeeeeeee; + /// \endcode + BBO_Never, + + /// Binary operations will either be all on the same line, or each operation + /// will have one line each. + /// \code + /// aaa + + /// bbbb * + /// ccccc - + /// ddddd + + /// eeeeeeeeeeeeeeee; + /// \endcode + BBO_OnePerLine, + + /// Binary operations of a particular precedence that exceed the column + /// limit will have one line each. + /// \code + /// aaa + + /// bbbb * ccccc - + /// ddddd + + /// eeeeeeeeeeeeeeee; + /// \endcode + BBO_RespectPrecedence + }; + + /// The break constructor initializers style to use. + /// \version 20 + BreakBinaryOperationsStyle BreakBinaryOperations; + /// Different ways to break initializers. enum BreakConstructorInitializersStyle : int8_t { /// Break constructor initializers before the colon and after the commas. @@ -5037,6 +5072,7 @@ struct FormatStyle { BreakBeforeConceptDeclarations == R.BreakBeforeConceptDeclarations && BreakBeforeInlineASMColon == R.BreakBeforeInlineASMColon && BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators && + BreakBinaryOperations == R.BreakBinaryOperations && BreakConstructorInitializers == R.BreakConstructorInitializers && BreakFunctionDefinitionParameters == R.BreakFunctionDefinitionParameters && diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index df86a774ba0f4f..43d246b7f82419 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -131,7 +131,8 @@ static bool startsSegmentOfBuilderTypeCall(const FormatToken &Tok) { // Returns \c true if \c Current starts a new parameter. static bool startsNextParameter(const FormatToken &Current, const FormatStyle &Style) { - const FormatToken &Previous = *Current.Previous; + assert(Current.Previous); + const auto &Previous = *Current.Previous; if (Current.is(TT_CtorInitializerComma) && Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma) { return true; @@ -146,6 +147,31 @@ static bool startsNextParameter(const FormatToken &Current, Style.BreakInheritanceList != FormatStyle::BILS_BeforeComma)); } +// Returns \c true if \c Token in an alignable binary operator +static bool isAlignableBinaryOperator(const FormatToken &Token) { + // No need to align binary operators that only have two operands. + bool HasTwoOperands = Token.OperatorIndex == 0 && !Token.NextOperator; + return Token.is(TT_BinaryOperator) && !HasTwoOperands && + Token.getPrecedence() > prec::Conditional && + Token.getPrecedence() < prec::PointerToMember; +} + +// Returns \c true if \c Current starts the next operand in a binary operation. +static bool startsNextOperand(const FormatToken &Current) { + assert(Current.Previous); + const auto &Previous = *Current.Previous; + return isAlignableBinaryOperator(Previous) && !Current.isTrailingComment(); +} + +// Returns \c true if \c Current is a binary operation that must break. +static bool mustBreakBinaryOperation(const FormatToken &Current, + const FormatStyle &Style) { + return Style.BreakBinaryOperations != FormatStyle::BBO_Never && + (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None + ? startsNextOperand + : isAlignableBinaryOperator)(Current); +} + static bool opensProtoMessageField(const FormatToken &LessTok, const FormatStyle &Style) { if (LessTok.isNot(tok::less)) @@ -869,6 +895,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, } if (CurrentState.AvoidBinPacking && startsNextParameter(Current, Style)) CurrentState.NoLineBreak = true; + if (mustBreakBinaryOperation(Current, Style)) + CurrentState.NoLineBreak = true; + if (startsSegmentOfBuilderTypeCall(Current) && State.Column > getNewLineColumn(State)) { CurrentState.ContainsUnwrappedBuilder = true; @@ -1235,6 +1264,9 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State, } } + if (mustBreakBinaryOperation(Current, Style)) + CurrentState.BreakBeforeParameter = true; + return Penalty; } diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 7fd42e46e0ccb7..5358b35c19de25 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -244,6 +244,16 @@ struct ScalarEnumerationTraits<FormatStyle::BreakBeforeInlineASMColonStyle> { } }; +template <> +struct ScalarEnumerationTraits<FormatStyle::BreakBinaryOperationsStyle> { + static void enumeration(IO &IO, + FormatStyle::BreakBinaryOperationsStyle &Value) { + IO.enumCase(Value, "Never", FormatStyle::BBO_Never); + IO.enumCase(Value, "OnePerLine", FormatStyle::BBO_OnePerLine); + IO.enumCase(Value, "RespectPrecedence", FormatStyle::BBO_RespectPrecedence); + } +}; + template <> struct ScalarEnumerationTraits<FormatStyle::BreakConstructorInitializersStyle> { static void @@ -968,6 +978,7 @@ template <> struct MappingTraits<FormatStyle> { Style.BreakBeforeInlineASMColon); IO.mapOptional("BreakBeforeTernaryOperators", Style.BreakBeforeTernaryOperators); + IO.mapOptional("BreakBinaryOperations", Style.BreakBinaryOperations); IO.mapOptional("BreakConstructorInitializers", Style.BreakConstructorInitializers); IO.mapOptional("BreakFunctionDefinitionParameters", @@ -1480,6 +1491,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.BreakBeforeConceptDeclarations = FormatStyle::BBCDS_Always; LLVMStyle.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline; LLVMStyle.BreakBeforeTernaryOperators = true; + LLVMStyle.BreakBinaryOperations = FormatStyle::BBO_Never; LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon; LLVMStyle.BreakFunctionDefinitionParameters = false; LLVMStyle.BreakInheritanceList = FormatStyle::BILS_BeforeColon; diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 7529c2e77950c5..9f79fa9fc516ca 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -3175,6 +3175,15 @@ class ExpressionParser { parse(Precedence + 1); int CurrentPrecedence = getCurrentPrecedence(); + if (Style.BreakBinaryOperations == FormatStyle::BBO_OnePerLine && + CurrentPrecedence > prec::Conditional && + CurrentPrecedence < prec::PointerToMember) { + // When BreakBinaryOperations is set to BreakAll, + // all operations will be on the same line or on individual lines. + // Override precedence to avoid adding fake parenthesis which could + // group operations of a diff erent precedence level on the same line + CurrentPrecedence = prec::Additive; + } if (Precedence == CurrentPrecedence && Current && Current->is(TT_SelectorName)) { diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index cc044153b7c9be..2ee0df99353ff5 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -404,6 +404,14 @@ TEST(ConfigParseTest, ParsesConfiguration) { CHECK_PARSE("BreakBeforeBinaryOperators: true", BreakBeforeBinaryOperators, FormatStyle::BOS_All); + Style.BreakBinaryOperations = FormatStyle::BBO_Never; + CHECK_PARSE("BreakBinaryOperations: OnePerLine", BreakBinaryOperations, + FormatStyle::BBO_OnePerLine); + CHECK_PARSE("BreakBinaryOperations: RespectPrecedence", BreakBinaryOperations, + FormatStyle::BBO_RespectPrecedence); + CHECK_PARSE("BreakBinaryOperations: Never", BreakBinaryOperations, + FormatStyle::BBO_Never); + Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon; CHECK_PARSE("BreakConstructorInitializers: BeforeComma", BreakConstructorInitializers, FormatStyle::BCIS_BeforeComma); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 2a754a29e81e73..bad1b1d662d133 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -27659,6 +27659,245 @@ TEST_F(FormatTest, SpaceBetweenKeywordAndLiteral) { verifyFormat("return sizeof \"5\";"); } +TEST_F(FormatTest, BreakBinaryOperations) { + auto Style = getLLVMStyleWithColumns(60); + EXPECT_EQ(Style.BreakBinaryOperations, FormatStyle::BBO_Never); + + // Logical operations + verifyFormat("if (condition1 && condition2) {\n" + "}", + Style); + + verifyFormat("if (condition1 && condition2 &&\n" + " (condition3 || condition4) && condition5 &&\n" + " condition6) {\n" + "}", + Style); + + verifyFormat("if (loooooooooooooooooooooongcondition1 &&\n" + " loooooooooooooooooooooongcondition2) {\n" + "}", + Style); + + // Arithmetic + verifyFormat("const int result = lhs + rhs;", Style); + + verifyFormat("const int result = loooooooongop1 + looooooooongop2 +\n" + " loooooooooooooooooooooongop3;", + Style); + + verifyFormat("result = longOperand1 + longOperand2 -\n" + " (longOperand3 + longOperand4) -\n" + " longOperand5 * longOperand6;", + Style); + + verifyFormat("const int result =\n" + " operand1 + operand2 - (operand3 + operand4);", + Style); + + Style.BreakBinaryOperations = FormatStyle::BBO_OnePerLine; + + // 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;", Style); + + verifyFormat("result = loooooooooooooooooooooongop1 +\n" + " loooooooooooooooooooooongop2 +\n" + " loooooooooooooooooooooongop3;", + Style); + + verifyFormat("const int result =\n" + " operand1 + operand2 - (operand3 + operand4);", + Style); + + verifyFormat("result = longOperand1 +\n" + " longOperand2 -\n" + " (longOperand3 + longOperand4) -\n" + " longOperand5 +\n" + " longOperand6;", + Style); + + verifyFormat("result = operand1 +\n" + " operand2 -\n" + " operand3 +\n" + " operand4 -\n" + " operand5 +\n" + " operand6;", + Style); + + // Ensure mixed precedence operations are handled properly + verifyFormat("result = op1 + op2 * op3 - op4;", Style); + + verifyFormat("result = operand1 +\n" + " operand2 /\n" + " operand3 +\n" + " operand4 /\n" + " operand5 *\n" + " operand6;", + Style); + + verifyFormat("result = operand1 *\n" + " operand2 -\n" + " operand3 *\n" + " operand4 -\n" + " operand5 +\n" + " operand6;", + Style); + + verifyFormat("result = operand1 *\n" + " (operand2 - operand3 * operand4) -\n" + " operand5 +\n" + " operand6;", + Style); + + verifyFormat("result = operand1.member *\n" + " (operand2.member() - operand3->mem * operand4) -\n" + " operand5.member() +\n" + " operand6->member;", + Style); + + Style.BreakBinaryOperations = FormatStyle::BBO_RespectPrecedence; + verifyFormat("result = op1 + op2 * op3 - op4;", Style); + + verifyFormat("result = operand1 +\n" + " operand2 / operand3 +\n" + " operand4 / operand5 * operand6;", + Style); + + verifyFormat("result = operand1 * operand2 -\n" + " operand3 * operand4 -\n" + " operand5 +\n" + " operand6;", + Style); + + verifyFormat("result = operand1 * (operand2 - operand3 * operand4) -\n" + " operand5 +\n" + " operand6;", + Style); + + verifyFormat("std::uint32_t a = byte_buffer[0] |\n" + " byte_buffer[1] << 8 |\n" + " byte_buffer[2] << 16 |\n" + " byte_buffer[3] << 24;", + Style); + + Style.BreakBinaryOperations = FormatStyle::BBO_OnePerLine; + Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment; + + // Logical operations + verifyFormat("if (condition1 && condition2) {\n" + "}", + Style); + + verifyFormat("if (loooooooooooooooooooooongcondition1\n" + " && loooooooooooooooooooooongcondition2) {\n" + "}", + Style); + + // Arithmetic + verifyFormat("const int result = lhs + rhs;", Style); + + verifyFormat("result = loooooooooooooooooooooongop1\n" + " + loooooooooooooooooooooongop2\n" + " + loooooooooooooooooooooongop3;", + Style); + + verifyFormat("const int result =\n" + " operand1 + operand2 - (operand3 + operand4);", + Style); + + verifyFormat("result = longOperand1\n" + " + longOperand2\n" + " - (longOperand3 + longOperand4)\n" + " - longOperand5\n" + " + longOperand6;", + Style); + + verifyFormat("result = operand1\n" + " + operand2\n" + " - operand3\n" + " + operand4\n" + " - operand5\n" + " + operand6;", + Style); + + // Ensure mixed precedence operations are handled properly + verifyFormat("result = op1 + op2 * op3 - op4;", Style); + + verifyFormat("result = operand1\n" + " + operand2\n" + " / operand3\n" + " + operand4\n" + " / operand5\n" + " * operand6;", + Style); + + verifyFormat("result = operand1\n" + " * operand2\n" + " - operand3\n" + " * operand4\n" + " - operand5\n" + " + operand6;", + Style); + + verifyFormat("result = operand1\n" + " * (operand2 - operand3 * operand4)\n" + " - operand5\n" + " + operand6;", + Style); + + verifyFormat("std::uint32_t a = byte_buffer[0]\n" + " | byte_buffer[1]\n" + " << 8\n" + " | byte_buffer[2]\n" + " << 16\n" + " | byte_buffer[3]\n" + " << 24;", + Style); + + Style.BreakBinaryOperations = FormatStyle::BBO_RespectPrecedence; + verifyFormat("result = op1 + op2 * op3 - op4;", Style); + + verifyFormat("result = operand1\n" + " + operand2 / operand3\n" + " + operand4 / operand5 * operand6;", + Style); + + verifyFormat("result = operand1 * operand2\n" + " - operand3 * operand4\n" + " - operand5\n" + " + operand6;", + Style); + + verifyFormat("result = operand1 * (operand2 - operand3 * operand4)\n" + " - operand5\n" + " + operand6;", + Style); + + verifyFormat("std::uint32_t a = byte_buffer[0]\n" + " | byte_buffer[1] << 8\n" + " | byte_buffer[2] << 16\n" + " | byte_buffer[3] << 24;", + Style); +} + } // namespace } // namespace test } // namespace format _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
