owenpan created this revision.
owenpan added a reviewer: mitchell-stellar.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, MyDeveloperDay.
owenpan requested review of this revision.
Since 3.8 or earlier, clang-format has been lumping all `#else`, `#elif`, etc
blocks together when doing whitespace replacements and causing consecutive
alignments across #else blocks.
Commit c077975
<https://reviews.llvm.org/rGc0779756a0c4cc84d9f98714734d47879701cc3d> partially
addressed the problem but also triggered "regressions".
This patch fixes the root cause of the problem and "reverts" c077975
<https://reviews.llvm.org/rGc0779756a0c4cc84d9f98714734d47879701cc3d> (except
for the unit tests).
Fixes https://github.com/llvm/llvm-project/issues/36070.
Fixes https://github.com/llvm/llvm-project/issues/55265.
Fixes https://github.com/llvm/llvm-project/issues/60721.
Fixes https://github.com/llvm/llvm-project/issues/61498.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D150057
Files:
clang/lib/Format/WhitespaceManager.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/FormatTestComments.cpp
Index: clang/unittests/Format/FormatTestComments.cpp
===================================================================
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2759,7 +2759,7 @@
// Checks an edge case in preprocessor handling.
// These comments should *not* be aligned
- EXPECT_NE( // change for EQ when fixed
+ EXPECT_EQ( // change for EQ when fixed
"#if FOO\n"
"#else\n"
"long a; // Line about a\n"
@@ -4316,7 +4316,7 @@
)",
format(R"(//
/\
-/
+/
)",
getLLVMStyleWithColumns(10)));
}
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6367,6 +6367,51 @@
"#endif\n"
"}",
Style);
+
+ verifyFormat("#if FOO\n"
+ "int a = 1;\n"
+ "#else\n"
+ "int ab = 2;\n"
+ "#endif\n"
+ "#ifdef BAR\n"
+ "int abc = 3;\n"
+ "#elifdef BAZ\n"
+ "int abcd = 4;\n"
+ "#endif",
+ Style);
+
+ verifyFormat("void f() {\n"
+ " if (foo) {\n"
+ "#if FOO\n"
+ " int a = 1;\n"
+ "#else\n"
+ " bool a = true;\n"
+ "#endif\n"
+ " int abc = 3;\n"
+ "#ifndef BAR\n"
+ " int abcd = 4;\n"
+ "#elif BAZ\n"
+ " bool ab = true;\n"
+ "#endif\n"
+ " }\n"
+ "}",
+ Style);
+
+ verifyFormat("void f() {\n"
+ "#if FOO\n"
+ " a = 1;\n"
+ "#else\n"
+ " ab = 2;\n"
+ "#endif\n"
+ "}\n"
+ "void g() {\n"
+ "#if BAR\n"
+ " abc = 3;\n"
+ "#elifndef BAZ\n"
+ " abcd = 4;\n"
+ "#endif\n"
+ "}",
+ Style);
}
TEST_F(FormatTest, FormatHashIfNotAtStartOfLine) {
Index: clang/lib/Format/WhitespaceManager.cpp
===================================================================
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -49,8 +49,16 @@
unsigned Spaces,
unsigned StartOfTokenColumn,
bool IsAligned, bool InPPDirective) {
- if (Tok.Finalized || (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg))
+ auto PPBranchDirectiveStartsLine = [&Tok] {
+ return Tok.is(tok::hash) && !Tok.Previous && Tok.Next &&
+ Tok.Next->isOneOf(tok::pp_if, tok::pp_ifdef, tok::pp_ifndef,
+ tok::pp_elif, tok::pp_elifdef, tok::pp_elifndef,
+ tok::pp_else, tok::pp_endif);
+ };
+ if ((Tok.Finalized && !PPBranchDirectiveStartsLine()) ||
+ (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg)) {
return;
+ }
Tok.setDecision((Newlines > 0) ? FD_Break : FD_Continue);
Changes.push_back(Change(Tok, /*CreateReplacement=*/true, Tok.WhitespaceRange,
Spaces, StartOfTokenColumn, Newlines, "", "",
@@ -522,13 +530,6 @@
? 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.
@@ -563,19 +564,8 @@
unsigned i = StartAt;
for (unsigned e = Changes.size(); i != e; ++i) {
- 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].indentAndNestingLevel() < IndentAndNestingLevel)
+ break;
if (Changes[i].NewlinesBefore != 0) {
CommasBeforeMatch = 0;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
