[clang] c5a4291 - [clang-format] Add BreakBinaryOperations configuration (#95013)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Aug 10 13:28:35 PDT 2024
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
More information about the cfe-commits
mailing list