mitchell-stellar created this revision.
mitchell-stellar added reviewers: HazardyKnusperkeks, curdeius, MyDeveloperDay.
mitchell-stellar added a project: clang-format.
Herald added a project: All.
mitchell-stellar requested review of this revision.
Herald added a project: clang.
Fixes https://github.com/llvm/llvm-project/issues/36070
clang-format makes multiple passes when #if/#else preprocessor blocks are
found. It will make one pass for normal code and code in the #if block, and
then it will make another pass for just the code in #else blocks. This often
results in invalid alignment inside the else blocks because they do not have
any scope or indentAndNestingLevel context from their surrounding tokens/lines.
This patch remedies that by caching any initial indentAndNestingLevel from a
second pass and not breaking/returning early when a scope change is detected.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D134042
Files:
clang/lib/Format/WhitespaceManager.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5743,6 +5743,139 @@
Style);
}
+TEST_F(FormatTest, FormatAlignInsidePreprocessorElseBlock) {
+ FormatStyle Style = getLLVMStyleWithColumns(40);
+ Style.AlignConsecutiveAssignments.Enabled = true;
+ Style.AlignConsecutiveDeclarations.Enabled = true;
+
+ // Test with just #if blocks.
+ {
+ const char *Expected = "void f1() {\n"
+ "#if 1\n"
+ " int foo = 1;\n"
+ " int foobar = 2;\n"
+ "#endif\n"
+ "}\n"
+ "\n"
+ "#if 1\n"
+ "int baz = 3;\n"
+ "#endif\n"
+ "\n"
+ "void f2() {\n"
+ "#if 1\n"
+ " char *foobarbaz = \"foobarbaz\";\n"
+ " int quux = 4;\n"
+ "}";
+ const char *ToFormat = "void f1() {\n"
+ "#if 1\n"
+ " int foo = 1;\n"
+ " int foobar = 2;\n"
+ "#endif\n"
+ "}\n"
+ "\n"
+ "#if 1\n"
+ "int baz = 3;\n"
+ "#endif\n"
+ "\n"
+ "void f2() {\n"
+ "#if 1\n"
+ " char *foobarbaz = \"foobarbaz\";\n"
+ " int quux = 4;\n"
+ "}";
+ EXPECT_EQ(Expected, format(ToFormat, Style));
+ EXPECT_EQ(Expected, format(Expected, Style));
+ }
+
+ // Test with just #else blocks.
+ {
+ const char *Expected = "void f1() {\n"
+ "#if 1\n"
+ "#else\n"
+ " int foo = 1;\n"
+ " int foobar = 2;\n"
+ "#endif\n"
+ "}\n"
+ "\n"
+ "#if 1\n"
+ "#else\n"
+ "int baz = 3;\n"
+ "#endif\n"
+ "\n"
+ "void f2() {\n"
+ "#if 1\n"
+ "#else\n"
+ " char *foobarbaz = \"foobarbaz\";\n"
+ " int quux = 4;\n"
+ "}";
+ const char *ToFormat = "void f1() {\n"
+ "#if 1\n"
+ "#else\n"
+ " int foo = 1;\n"
+ " int foobar = 2;\n"
+ "#endif\n"
+ "}\n"
+ "\n"
+ "#if 1\n"
+ "#else\n"
+ "int baz = 3;\n"
+ "#endif\n"
+ "\n"
+ "void f2() {\n"
+ "#if 1\n"
+ "#else\n"
+ " char *foobarbaz = \"foobarbaz\";\n"
+ " int quux = 4;\n"
+ "}";
+ EXPECT_EQ(Expected, format(ToFormat, Style));
+ EXPECT_EQ(Expected, format(Expected, Style));
+ }
+
+ // Test with a mix of #if and #else blocks.
+ {
+ const char *Expected = "void f1() {\n"
+ "#if 1\n"
+ "#else\n"
+ " int foo = 1;\n"
+ " int foobar = 2;\n"
+ " // prevent alignment with #else in f2\n"
+ "#endif\n"
+ "}\n"
+ "\n"
+ "#if 1\n"
+ "int baz = 3;\n"
+ "#endif\n"
+ "\n"
+ "void f2() {\n"
+ "#if 1\n"
+ "#else\n"
+ " char *foobarbaz = \"foobarbaz\";\n"
+ " int quux = 4;\n"
+ "}";
+ const char *ToFormat = "void f1() {\n"
+ "#if 1\n"
+ "#else\n"
+ " int foo = 1;\n"
+ " int foobar = 2;\n"
+ " // prevent alignment with #else in f2\n"
+ "#endif\n"
+ "}\n"
+ "\n"
+ "#if 1\n"
+ "int baz = 3;\n"
+ "#endif\n"
+ "\n"
+ "void f2() {\n"
+ "#if 1\n"
+ "#else\n"
+ " char *foobarbaz = \"foobarbaz\";\n"
+ " int quux = 4;\n"
+ "}";
+ EXPECT_EQ(Expected, format(ToFormat, Style));
+ EXPECT_EQ(Expected, format(Expected, Style));
+ }
+
+}
+
TEST_F(FormatTest, FormatHashIfNotAtStartOfLine) {
verifyFormat("{\n { a #c; }\n}");
}
Index: clang/lib/Format/WhitespaceManager.cpp
===================================================================
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -522,6 +522,13 @@
? Changes[StartAt].indentAndNestingLevel()
: std::tuple<unsigned, unsigned, unsigned>();
+ // Keep track if the first token has a non-zero indent and nesting level.
+ // This can happen when aligning the contents of "#else" preprocessor blocks,
+ // which is done separately.
+ bool HasInitialIndentAndNesting =
+ StartAt == 0 &&
+ IndentAndNestingLevel > std::tuple<unsigned, unsigned, unsigned>();
+
// Keep track of the number of commas before the matching tokens, we will only
// align a sequence of matching tokens if they are preceded by the same number
// of commas.
@@ -556,8 +563,19 @@
unsigned i = StartAt;
for (unsigned e = Changes.size(); i != e; ++i) {
- if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel)
- break;
+ if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel) {
+ if (!HasInitialIndentAndNesting)
+ break;
+ // The contents of preprocessor blocks are aligned separately.
+ // If the initial preprocessor block is indented or nested (e.g. it's in
+ // a function), do not align and exit after finishing this scope block.
+ // Instead, align, and then lower the baseline indent and nesting level
+ // in order to continue aligning subsequent blocks.
+ EndOfSequence = i;
+ AlignCurrentSequence();
+ IndentAndNestingLevel =
+ Changes[i].indentAndNestingLevel(); // new baseline
+ }
if (Changes[i].NewlinesBefore != 0) {
CommasBeforeMatch = 0;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
