-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[clang-format] Don't confuse initializer equal signs in for loops #77712
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang-format] Don't confuse initializer equal signs in for loops #77712
Conversation
clang-format has logic to align declarations of multiple variables of the same type, aligning them at the equals sign. This logic is applied in for loops as well. However, this alignment logic also erroneously affected the equals signs of designated initializers. This patch forbids alignment if the token 2 tokens back from the equals sign is a designated initializer period. Fixes llvm#73902
@llvm/pr-subscribers-clang-format Author: Emilia Kond (rymiel) Changesclang-format has logic to align declarations of multiple variables of the same type, aligning them at the equals sign. This logic is applied in for loops as well. However, this alignment logic also erroneously affected the equals signs of designated initializers. This patch forbids alignment if the token 2 tokens back from the equals sign is a designated initializer period. Fixes #73902 Full diff: https://github.com/llvm/llvm-project/pull/77712.diff 2 Files Affected:
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 102504182c4505..ce6c17c3b7d927 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -703,7 +703,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
if (Current.is(tok::equal) &&
(State.Line->First->is(tok::kw_for) || Current.NestingLevel == 0) &&
- CurrentState.VariablePos == 0) {
+ CurrentState.VariablePos == 0 &&
+ (!Previous.Previous ||
+ Previous.Previous->isNot(TT_DesignatedInitializerPeriod))) {
CurrentState.VariablePos = State.Column;
// Move over * and & if they are bound to the variable name.
const FormatToken *Tok = &Previous;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 25ef5c680af862..01678a4971caae 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -4944,6 +4944,18 @@ TEST_F(FormatTest, DesignatedInitializers) {
" [3] = cccccccccccccccccccccccccccccccccccccc,\n"
" [4] = dddddddddddddddddddddddddddddddddddddd,\n"
" [5] = eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee};");
+
+ verifyFormat("for (const TestCase &test_case : {\n"
+ " TestCase{\n"
+ " .a = 1,\n"
+ " .b = 1,\n"
+ " },\n"
+ " TestCase{\n"
+ " .a = 2,\n"
+ " .b = 2,\n"
+ " },\n"
+ " }) {\n"
+ "}\n");
}
TEST_F(FormatTest, BracedInitializerIndentWidth) {
|
CurrentState.VariablePos == 0) { | ||
CurrentState.VariablePos == 0 && | ||
(!Previous.Previous || | ||
Previous.Previous->isNot(TT_DesignatedInitializerPeriod))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might need to search further back if the element being initialized is an array.
struct { int x[2]; } a[] = { [0].x = {1,1}, [1].x[0] = 42};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a for loop, but similar code should be possible in a for loop initialization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we handle that in seperate issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, see if anyone complains anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me I'm happy if we keep it to this use case only.
CurrentState.VariablePos == 0) { | ||
CurrentState.VariablePos == 0 && | ||
(!Previous.Previous || | ||
Previous.Previous->isNot(TT_DesignatedInitializerPeriod))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we handle that in seperate issue?
clang-format has logic to align declarations of multiple variables of the same type, aligning them at the equals sign. This logic is applied in for loops as well. However, this alignment logic also erroneously affected the equals signs of designated initializers.
This patch forbids alignment if the token 2 tokens back from the equals sign is a designated initializer period.
Fixes #73902