[clang-format] Add AlignAfterOpenBracketOptions #108332
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-format @llvm/pr-subscribers-clang Author: Gedare Bloom (gedare) ChangesIntroduce sub-options for Fixes #67738 Patch is 29.57 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/108332.diff 8 Files Affected:
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index a427d7cd40fcdd..62462c3202bee0 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -246,6 +246,66 @@ the configuration (without a prefix: ``Auto``).
+.. _AlignAfterOpenBracketBreak:
+
+**AlignAfterOpenBracketBreak** (``AlignAfterOpenBracketCustom``) :versionbadge:`clang-format 20` :ref:`¶ <AlignAfterOpenBracketBreak>`
+ Control of when ``AlignAfterOpenBracket`` breaks an opening bracket.
+
+ If ``AlignAfterOpenBracket`` is set to ``AlwaysBreak`` or ``BlockIndent``,
+ use this to specify how different cases of breaking the opening brackets
+ should be handled. Otherwise, this is ignored. Setting any of these to
+ ``false`` will cause them to not break. At least one of these must be set
+ to ``true``, otherwise a default (backward compatible) breaking behavior
+ is used. This is ignored for ``Align`` and ``DontAlign``.
+
+ .. code-block:: c++
+
+ # Example of usage:
+ AlignAfterOpenBracket: AlwaysBreak
+ AlignAfterOpenBracketBreak:
+ InIfConditionalStatements: true
+ InOtherConditionalStatements: false
+ Other: true
+
+ Nested configuration flags:
+
+ Precise control over breaking the opening bracket of
+ ``AlignAfterOpenBracket``.
+
+ .. code-block:: c++
+
+ # Should be declared this way:
+ AlignAfterOpenBracketBreak:
+ InIfConditionalStatements: true
+ InOtherConditionalStatements: false
+ Other: true
+
+ * ``bool InIfConditionalStatements`` Break inside if/else if statements.
+
+ .. code-block:: c++
+
+ true: false:
+ if constexpr ( vs. if constexpr (a ||
+ a || b) b)
+
+ * ``bool InOtherConditionalStatements`` Break inside conditional statements not covered by preceding options.
+ (``for/while/switch...``).
+
+ .. code-block:: c++
+
+ true: false:
+ while ( vs. while (a &&
+ a && b ) { b) {
+
+ * ``bool Other`` Break inside brackets not covered by preceding options.
+
+ .. code-block:: c++
+
+ true: false:
+ someLongFunction( vs. someLongFunction(argument1,
+ argument1, argument2); argument2);
+
+
.. _AlignArrayOfStructures:
**AlignArrayOfStructures** (``ArrayInitializerAlignmentStyle``) :versionbadge:`clang-format 13` :ref:`¶ <AlignArrayOfStructures>`
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9860b25f2e7fa6..2141f6b2ad1d41 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -502,6 +502,8 @@ clang-format
------------
- Adds ``BreakBinaryOperations`` option.
+- Adds ``AlignAfterOpenBracketBreak`` sub-options for better control of
+ ``AlignAfterOpenBracket`` with ``AlwaysBreak`` or ``BlockIndent`` modes.
libclang
--------
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index d8b62c7652a0f6..6e173d6d2b459f 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -106,6 +106,78 @@ struct FormatStyle {
/// \version 3.8
BracketAlignmentStyle AlignAfterOpenBracket;
+ /// Precise control over breaking the opening bracket of
+ /// ``AlignAfterOpenBracket``.
+ /// \code
+ /// # Should be declared this way:
+ /// AlignAfterOpenBracketBreak:
+ /// InIfConditionalStatements: true
+ /// InOtherConditionalStatements: false
+ /// Other: true
+ /// \endcode
+ struct AlignAfterOpenBracketCustom {
+ /// Break inside if/else if statements.
+ /// \code
+ /// true: false:
+ /// if constexpr ( vs. if constexpr (a ||
+ /// a || b) b)
+ /// \endcode
+ bool InIfConditionalStatements;
+ /// Break inside conditional statements not covered by preceding options.
+ /// (``for/while/switch...``).
+ /// \code
+ /// true: false:
+ /// while ( vs. while (a &&
+ /// a && b ) { b) {
+ /// \endcode
+ bool InOtherConditionalStatements;
+ /// Break inside brackets not covered by preceding options.
+ /// \code
+ /// true: false:
+ /// someLongFunction( vs. someLongFunction(argument1,
+ /// argument1, argument2); argument2);
+ /// \endcode
+ bool Other;
+
+ AlignAfterOpenBracketCustom()
+ : InIfConditionalStatements(false), InOtherConditionalStatements(false),
+ Other(false) {}
+
+ AlignAfterOpenBracketCustom(bool InIfConditionalStatements,
+ bool InOtherConditionalStatements, bool Other)
+ : InIfConditionalStatements(InIfConditionalStatements),
+ InOtherConditionalStatements(InOtherConditionalStatements),
+ Other(Other) {}
+
+ bool operator==(const AlignAfterOpenBracketCustom &R) const {
+ return InIfConditionalStatements == R.InIfConditionalStatements &&
+ InOtherConditionalStatements == R.InOtherConditionalStatements &&
+ Other == R.Other;
+ }
+ bool operator!=(const AlignAfterOpenBracketCustom &R) const {
+ return !(*this == R);
+ }
+ };
+
+ /// Control of when ``AlignAfterOpenBracket`` breaks an opening bracket.
+ ///
+ /// If ``AlignAfterOpenBracket`` is set to ``AlwaysBreak`` or ``BlockIndent``,
+ /// use this to specify how different cases of breaking the opening brackets
+ /// should be handled. Otherwise, this is ignored. Setting any of these to
+ /// ``false`` will cause them to not break. At least one of these must be set
+ /// to ``true``, otherwise a default (backward compatible) breaking behavior
+ /// is used. This is ignored for ``Align`` and ``DontAlign``.
+ /// \code
+ /// # Example of usage:
+ /// AlignAfterOpenBracket: AlwaysBreak
+ /// AlignAfterOpenBracketBreak:
+ /// InIfConditionalStatements: true
+ /// InOtherConditionalStatements: false
+ /// Other: true
+ /// \endcode
+ /// \version 20
+ AlignAfterOpenBracketCustom AlignAfterOpenBracketBreak;
+
/// Different style for aligning array initializers.
enum ArrayInitializerAlignmentStyle : int8_t {
/// Align array column and left justify the columns e.g.:
@@ -5060,6 +5132,7 @@ struct FormatStyle {
bool operator==(const FormatStyle &R) const {
return AccessModifierOffset == R.AccessModifierOffset &&
AlignAfterOpenBracket == R.AlignAfterOpenBracket &&
+ AlignAfterOpenBracketBreak == R.AlignAfterOpenBracketBreak &&
AlignArrayOfStructures == R.AlignArrayOfStructures &&
AlignConsecutiveAssignments == R.AlignConsecutiveAssignments &&
AlignConsecutiveBitFields == R.AlignConsecutiveBitFields &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index f29f8796ea9290..673e8ca54ddc03 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -795,6 +795,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
// parenthesis by disallowing any further line breaks if there is no line
// break after the opening parenthesis. Don't break if it doesn't conserve
// columns.
+ auto IsOtherConditional = [](const FormatToken &Tok) {
+ return Tok.isOneOf(tok::kw_for, tok::kw_while, tok::kw_switch);
+ };
auto IsOpeningBracket = [&](const FormatToken &Tok) {
auto IsStartOfBracedList = [&]() {
return Tok.is(tok::l_brace) && Tok.isNot(BK_Block) &&
@@ -807,9 +810,11 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
if (!Tok.Previous)
return true;
if (Tok.Previous->isIf())
- return Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak;
- return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
- tok::kw_switch);
+ return Style.AlignAfterOpenBracketBreak.InIfConditionalStatements;
+ if (IsOtherConditional(*Tok.Previous))
+ return Style.AlignAfterOpenBracketBreak.InOtherConditionalStatements;
+ return !Tok.Previous->is(TT_CastRParen) &&
+ Style.AlignAfterOpenBracketBreak.Other;
};
auto IsFunctionCallParen = [](const FormatToken &Tok) {
return Tok.is(tok::l_paren) && Tok.ParameterCount > 0 && Tok.Previous &&
@@ -844,10 +849,15 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
Tok.isOneOf(tok::ellipsis, Keywords.kw_await))) {
return true;
}
- const auto *Previous = Tok.Previous;
- if (!Previous || (!Previous->isOneOf(TT_FunctionDeclarationLParen,
- TT_LambdaDefinitionLParen) &&
- !IsFunctionCallParen(*Previous))) {
+ const auto *Previous = TokAfterLParen.Previous;
+ assert(Previous); // IsOpeningBracket(Previous)
+ if (Previous->Previous && (Previous->Previous->isIf() ||
+ IsOtherConditional(*Previous->Previous))) {
+ return false;
+ }
+ if (!Previous->isOneOf(TT_FunctionDeclarationLParen,
+ TT_LambdaDefinitionLParen) &&
+ !IsFunctionCallParen(*Previous)) {
return true;
}
if (IsOpeningBracket(Tok) || IsInTemplateString(Tok))
@@ -1225,8 +1235,21 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
}
if (PreviousNonComment && PreviousNonComment->is(tok::l_paren)) {
- CurrentState.BreakBeforeClosingParen =
- Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent;
+ CurrentState.BreakBeforeClosingParen = false;
+ if (Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) {
+ auto Previous = PreviousNonComment->Previous;
+ if (Previous && Previous->isIf()) {
+ CurrentState.BreakBeforeClosingParen =
+ Style.AlignAfterOpenBracketBreak.InIfConditionalStatements;
+ } else if (Previous && Previous->isOneOf(tok::kw_for, tok::kw_while,
+ tok::kw_switch)) {
+ CurrentState.BreakBeforeClosingParen =
+ Style.AlignAfterOpenBracketBreak.InOtherConditionalStatements;
+ } else {
+ CurrentState.BreakBeforeClosingParen =
+ Style.AlignAfterOpenBracketBreak.Other;
+ }
+ }
}
if (CurrentState.AvoidBinPacking) {
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index d2463b892fbb96..79069cb1678c91 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -216,6 +216,16 @@ template <> struct ScalarEnumerationTraits<FormatStyle::BracketAlignmentStyle> {
}
};
+template <> struct MappingTraits<FormatStyle::AlignAfterOpenBracketCustom> {
+ static void mapping(IO &IO, FormatStyle::AlignAfterOpenBracketCustom &Value) {
+ IO.mapOptional("InIfConditionalStatements",
+ Value.InIfConditionalStatements);
+ IO.mapOptional("InOtherConditionalStatements",
+ Value.InOtherConditionalStatements);
+ IO.mapOptional("Other", Value.Other);
+ }
+};
+
template <>
struct ScalarEnumerationTraits<
FormatStyle::BraceWrappingAfterControlStatementStyle> {
@@ -922,6 +932,8 @@ template <> struct MappingTraits<FormatStyle> {
IO.mapOptional("AccessModifierOffset", Style.AccessModifierOffset);
IO.mapOptional("AlignAfterOpenBracket", Style.AlignAfterOpenBracket);
+ IO.mapOptional("AlignAfterOpenBracketBreak",
+ Style.AlignAfterOpenBracketBreak);
IO.mapOptional("AlignArrayOfStructures", Style.AlignArrayOfStructures);
IO.mapOptional("AlignConsecutiveAssignments",
Style.AlignConsecutiveAssignments);
@@ -1155,6 +1167,18 @@ template <> struct MappingTraits<FormatStyle> {
IO.mapOptional("WhitespaceSensitiveMacros",
Style.WhitespaceSensitiveMacros);
+ // If AlignAfterOpenBracket was specified but AlignAfterOpenBracketBreak
+ // was not, initialize the latter for backwards compatibility.
+ if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak ||
+ Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) &&
+ Style.AlignAfterOpenBracketBreak ==
+ FormatStyle::AlignAfterOpenBracketCustom()) {
+ if (Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak)
+ Style.AlignAfterOpenBracketBreak.InIfConditionalStatements = true;
+ Style.AlignAfterOpenBracketBreak.InOtherConditionalStatements = false;
+ Style.AlignAfterOpenBracketBreak.Other = true;
+ }
+
// If AlwaysBreakAfterDefinitionReturnType was specified but
// BreakAfterReturnType was not, initialize the latter from the former for
// backwards compatibility.
@@ -1438,6 +1462,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
FormatStyle LLVMStyle;
LLVMStyle.AccessModifierOffset = -2;
LLVMStyle.AlignAfterOpenBracket = FormatStyle::BAS_Align;
+ LLVMStyle.AlignAfterOpenBracketBreak = {};
LLVMStyle.AlignArrayOfStructures = FormatStyle::AIAS_None;
LLVMStyle.AlignConsecutiveAssignments = {};
LLVMStyle.AlignConsecutiveAssignments.AcrossComments = false;
@@ -1750,6 +1775,7 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
GoogleStyle.SpacesBeforeTrailingComments = 1;
} else if (Language == FormatStyle::LK_JavaScript) {
GoogleStyle.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+ GoogleStyle.AlignAfterOpenBracketBreak = {true, false, true};
GoogleStyle.AlignOperands = FormatStyle::OAS_DontAlign;
GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
// TODO: still under discussion whether to switch to SLS_All.
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index dfa703aed0d34d..caa840f461da8f 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -6259,7 +6259,13 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
if (Next && Next->is(tok::l_paren))
return false;
const FormatToken *Previous = Right.MatchingParen->Previous;
- return !(Previous && (Previous->is(tok::kw_for) || Previous->isIf()));
+ if (!Previous)
+ return true;
+ if (Previous->isOneOf(tok::kw_for, tok::kw_while, tok::kw_switch))
+ return Style.AlignAfterOpenBracketBreak.InOtherConditionalStatements;
+ if (Previous->isIf())
+ return Style.AlignAfterOpenBracketBreak.InIfConditionalStatements;
+ return Style.AlignAfterOpenBracketBreak.Other;
}
if (Left.isOneOf(tok::r_paren, TT_TrailingAnnotation) &&
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index b8bdfaaa74e10e..c46a3e277e9134 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -203,6 +203,11 @@ TEST(ConfigParseTest, ParsesConfigurationBools) {
CHECK_PARSE_BOOL(SpaceBeforeSquareBrackets);
CHECK_PARSE_BOOL(VerilogBreakBetweenInstancePorts);
+ CHECK_PARSE_NESTED_BOOL(AlignAfterOpenBracketBreak,
+ InIfConditionalStatements);
+ CHECK_PARSE_NESTED_BOOL(AlignAfterOpenBracketBreak,
+ InOtherConditionalStatements);
+ CHECK_PARSE_NESTED_BOOL(AlignAfterOpenBracketBreak, Other);
CHECK_PARSE_NESTED_BOOL(AlignConsecutiveShortCaseStatements, Enabled);
CHECK_PARSE_NESTED_BOOL(AlignConsecutiveShortCaseStatements,
AcrossEmptyLines);
@@ -500,6 +505,13 @@ TEST(ConfigParseTest, ParsesConfiguration) {
FormatStyle::BAS_DontAlign);
CHECK_PARSE("AlignAfterOpenBracket: true", AlignAfterOpenBracket,
FormatStyle::BAS_Align);
+ Style.AlignAfterOpenBracket = FormatStyle::BAS_Align;
+ Style.AlignAfterOpenBracketBreak = {};
+ CHECK_PARSE("AlignAfterOpenBracket: AlwaysBreak", AlignAfterOpenBracketBreak,
+ FormatStyle::AlignAfterOpenBracketCustom(true, false, true));
+ Style.AlignAfterOpenBracketBreak = {};
+ CHECK_PARSE("AlignAfterOpenBracket: BlockIndent", AlignAfterOpenBracketBreak,
+ FormatStyle::AlignAfterOpenBracketCustom(false, false, true));
Style.AlignEscapedNewlines = FormatStyle::ENAS_Left;
CHECK_PARSE("AlignEscapedNewlines: DontAlign", AlignEscapedNewlines,
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 5ebf0d7068dd6c..641b685127f2e4 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -5072,6 +5072,7 @@ TEST_F(FormatTest, BracedInitializerIndentWidth) {
auto Style = getLLVMStyleWithColumns(60);
Style.BinPackArguments = true;
Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+ Style.AlignAfterOpenBracketBreak = {true, false, true};
Style.BracedInitializerIndentWidth = 6;
// Non-initializing braces are unaffected by BracedInitializerIndentWidth.
@@ -7320,6 +7321,7 @@ TEST_F(FormatTest, ExpressionIndentationBreakingBeforeOperators) {
Style = getLLVMStyleWithColumns(20);
Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
Style.BinPackParameters = FormatStyle::BPPS_OnePerLine;
+ Style.AlignAfterOpenBracketBreak = {true, false, true};
Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
Style.ContinuationIndentWidth = 2;
verifyFormat("struct Foo {\n"
@@ -7989,12 +7991,14 @@ TEST_F(FormatTest, AllowAllArgumentsOnNextLineDontAlign) {
// However, BAS_AlwaysBreak and BAS_BlockIndent should take precedence over
// AllowAllArgumentsOnNextLine.
Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+ Style.AlignAfterOpenBracketBreak = {true, false, true};
verifyFormat(StringRef("functionCall(\n"
" paramA, paramB, paramC);\n"
"void functionDecl(\n"
" int A, int B, int C);"),
Input, Style);
Style.AlignAfterOpenBracket = FormatStyle::BAS_BlockIndent;
+ Style.AlignAfterOpenBracketBreak = {false, false, true};
verifyFormat("functionCall(\n"
" paramA, paramB, paramC\n"
");\n"
@@ -8007,6 +8011,7 @@ TEST_F(FormatTest, AllowAllArgumentsOnNextLineDontAlign) {
// first argument.
Style.AllowAllArgumentsOnNextLine = true;
Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+ Style.AlignAfterOpenBracketBreak = {true, false, true};
verifyFormat(StringRef("functionCall(\n"
" paramA, paramB, paramC);\n"
"void functionDecl(\n"
@@ -8928,6 +8933,7 @@ TEST_F(FormatTest, FormatsOneParameterPerLineIfNecessary) {
TEST_F(FormatTest, FormatsDeclarationBreakAlways) {
FormatStyle BreakAlways = getGoogleStyle();
BreakAlways.BinPackParameters = FormatStyle::BPPS_AlwaysOnePerLine;
+ BreakAlways.AlignAfterOpenBracketBreak = {true, false, true};
verifyFormat("void f(int a,\n"
" int b);",
BreakAlways);
@@ -8956,6 +8962,7 @@ TEST_F(FormatTest, FormatsDeclarationBreakAlways) {
TEST_F(FormatTest, FormatsDefinitionBreakAlways) {
FormatStyle BreakAlways = getGoogleStyle();
BreakAlways.BinPackParameters = FormatStyle::BPPS_AlwaysOnePerLine;
+ BreakAlways.AlignAfterOpenBracketBreak = {true, false, true};
verifyFormat("void f(int a,\n"
" int b) {\n"
" f(a, b);\n"
@@ -9346,6 +9353,7 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) {
Style.ColumnLimit = 80;
Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+ Style.AlignAfterOpenBracketBreak = {true, false, true};
Style.BinPackArguments = false;
Style.BinPackParameters = FormatStyle::BPPS_OnePerLine;
verifyFormat("void aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
@@ -9397,6 +9405,7 @@ TEST_F(FormatTest, AlignsAfterOpenBracket) {
Style);
Style.AlignAfterOpenBracket = FormatStyle::BAS_BlockIndent;
+ Style.AlignAfterOpenBracketBreak = {false, false, true};
Style.BinPackArgum...
[truncated]
|
|
Rebased to main to pick up recent regression fixes BlockIndent/AlwaysBreak. |
gedare
force-pushed
the
67738
branch
3 times, most recently
from
8f17675 to
6a4640f
Compare
|
Rebased to main. This PR addresses several long-standing issues. It would be great to get a review. |
Adding a
My concern with a At the moment, these alignment and breaking options are fairly complex and bug-prone. |
|
There was a problem between this implementation and the fix in #119989 that I needed to figure out. Handling block indentation within template strings needed to be split in two cases, one case to limit the check for a template string to the current scope for determining how much to indent, and one case to search beyond the current scope for an enclosing template string. |
HazardyKnusperkeks
approved these changes
Jan 29, 2025
|
Rebased to |
|
Was there a change which I should look at again? I find the GitHub UI for reviews terrible. If it was just a rebase my vote stands. |
@HazardyKnusperkeks I think the only changes since your reviewer were to bump to version 21 and regenerate the files. Otherwise this was just a rebase and conflict fix. |
|
I prefer that we limit this to breaking after the left parenthesis of a control statement, with true/false or Always, Never, Multiline, BlockIndent, etc. |
if I understand you correctly, you would like a new style option added for setting break option for all control statements in the same way? |
Yes. |
This is basically re-written as requested. The Alignment is determined by the value of AlignAfterOpenBracket, except that because there is a forced break with |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions cpp,h -- 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.cppView the diff from clang-format here.diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 08558d655..f4e48b7e3 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -834,16 +834,15 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
/* For backward compatibility, use AlignAfterOpenBracket
* in case AlignAfterControlStatement is not initialized */
return Style.AlignAfterControlStatement == FormatStyle::BACSS_MultiLine ||
- (Style.AlignAfterControlStatement == FormatStyle::BACSS_Default &&
- Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak);
+ (Style.AlignAfterControlStatement == FormatStyle::BACSS_Default &&
+ Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak);
}
- if (IsOtherConditional(*Tok.Previous)) {
+ if (IsOtherConditional(*Tok.Previous))
return Style.AlignAfterControlStatement == FormatStyle::BACSS_MultiLine;
- }
if (Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak ||
- Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) {
+ Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) {
return !Tok.Previous->is(TT_CastRParen) &&
- !(Style.isJavaScript() && Tok.is(Keywords.kw_await));
+ !(Style.isJavaScript() && Tok.is(Keywords.kw_await));
}
return false;
};
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index e6a82a093..cd7e4cb06 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -202,8 +202,10 @@ template <> struct MappingTraits<FormatStyle::BraceWrappingFlags> {
}
};
-template <> struct ScalarEnumerationTraits<FormatStyle::BreakAfterControlStatementStyle> {
- static void enumeration(IO &IO, FormatStyle::BreakAfterControlStatementStyle &Value) {
+template <>
+struct ScalarEnumerationTraits<FormatStyle::BreakAfterControlStatementStyle> {
+ static void enumeration(IO &IO,
+ FormatStyle::BreakAfterControlStatementStyle &Value) {
IO.enumCase(Value, "Default", FormatStyle::BACSS_Default);
IO.enumCase(Value, "MultiLine", FormatStyle::BACSS_MultiLine);
IO.enumCase(Value, "No", FormatStyle::BACSS_No);
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 39ddb63d4..881d40abf 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -6221,7 +6221,8 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
const FormatToken *Previous = Right.MatchingParen->Previous;
if (!Previous)
return true;
- if (Previous->isIf() || Previous->isOneOf(tok::kw_for, tok::kw_while, tok::kw_switch)) {
+ if (Previous->isIf() ||
+ Previous->isOneOf(tok::kw_for, tok::kw_while, tok::kw_switch)) {
return Style.AlignAfterControlStatement == FormatStyle::BACSS_MultiLine;
}
return true;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 7841caefd..a314a0bff 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -9797,7 +9797,6 @@ TEST_F(FormatTest, AlignAfterConditionalStatements) {
"}",
Style);
-
Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
Style.AlignAfterControlStatement = FormatStyle::BACSS_MultiLine;
|
|
When I updated, my build target for I'm trying to figure out what changed or how to modify my workflow. I think I ran this PR with stale tests. |
See #134196 (comment). |
Introduce new style option to allow overriding the breaking after the opening parenthesis for control statements (if/for/while/switch). Fixes llvm#67738. Fixes llvm#79176. Fixes llvm#80123.
Why would I'm not sure if we should add this option if it's too complicated to understand. WDYT @mydeveloperday? |
owenca
reviewed
May 1, 2025
| @@ -9694,6 +9694,304 @@ TEST_F(FormatTest, ParenthesesAndOperandAlignment) { | |||
| Style); | |||
| } | |||
|
|
|||
| TEST_F(FormatTest, AlignAfterConditionalStatements) { | |||
There was a problem hiding this comment.
Suggested change
| TEST_F(FormatTest, AlignAfterConditionalStatements) { | |
| TEST_F(FormatTest, AlignAfterControlStatements) { |
Can you move this to a new file if all these new test cases are necessary and can't be shortened?
Comment on lines
+69
to
+76
| /// Use the default behavior. | ||
| BACSS_Default, | ||
| /// Force break after the left parenthesis of a control statement only | ||
| /// when the expression exceeds the column limit, and align on the | ||
| /// ``ContinuationIndentWidth``. | ||
| BACSS_MultiLine, | ||
| /// Do not force a break after the control statment. | ||
| BACSS_No, |
Unfortunately, We could certainly make the distinction between breaking and alignment more explicit, as mentioned at #80049 That's really what this PR is doing, it is adding an option to break the start of the control expression. The alignment gets inherited from the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduce sub-options for
AlignAfterOpenBracketBreakto allow for control ofAlignAfterOpenBracketwithAlwaysBreakandBlockIndentselectively forifconditional statements (as currently supported), other conditional statements (for/while/switch), and other statements.Fixes #67738
Fixes #79176
Fixes #80123