[clang] a4c87f8 - [clang-format] Fix consecutive alignments in #else blocks
Owen Pan via cfe-commits
cfe-commits at lists.llvm.org
Mon May 8 19:45:54 PDT 2023
Author: Owen Pan
Date: 2023-05-08T19:45:30-07:00
New Revision: a4c87f8ccaccc76fd7d1c6c2e639ca84b9ec7794
URL: https://github.com/llvm/llvm-project/commit/a4c87f8ccaccc76fd7d1c6c2e639ca84b9ec7794
DIFF: https://github.com/llvm/llvm-project/commit/a4c87f8ccaccc76fd7d1c6c2e639ca84b9ec7794.diff
LOG: [clang-format] Fix consecutive alignments in #else blocks
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 partially addressed the problem but also triggered
"regressions".
This patch fixes the root cause of the problem and "reverts" c077975
(except for the unit tests).
Fixes #36070.
Fixes #55265.
Fixes #60721.
Fixes #61498.
Differential Revision: https://reviews.llvm.org/D150057
Added:
Modified:
clang/lib/Format/WhitespaceManager.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/FormatTestComments.cpp
Removed:
################################################################################
diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index d2be3b3a77df4..040b9536b6305 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -49,8 +49,16 @@ void WhitespaceManager::replaceWhitespace(FormatToken &Tok, unsigned Newlines,
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 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
? 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 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
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;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 099a5cb913643..dc673934a3f1b 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -6367,6 +6367,51 @@ TEST_F(FormatTest, FormatAlignInsidePreprocessorElseBlock) {
"#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 abcd = 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) {
diff --git a/clang/unittests/Format/FormatTestComments.cpp b/clang/unittests/Format/FormatTestComments.cpp
index 60e045059be86..6e5b5b2d68d81 100644
--- a/clang/unittests/Format/FormatTestComments.cpp
+++ b/clang/unittests/Format/FormatTestComments.cpp
@@ -2759,7 +2759,7 @@ TEST_F(FormatTestComments, AlignTrailingComments) {
// Checks an edge case in preprocessor handling.
// These comments should *not* be aligned
- EXPECT_NE( // change for EQ when fixed
+ EXPECT_EQ(
"#if FOO\n"
"#else\n"
"long a; // Line about a\n"
@@ -4316,7 +4316,7 @@ TEST_F(FormatTestComments, SplitCommentIntroducers) {
)",
format(R"(//
/\
-/
+/
)",
getLLVMStyleWithColumns(10)));
}
More information about the cfe-commits
mailing list