Skip to content

Commit

Permalink
[clang-format] Fix consecutive alignments in #else blocks
Browse files Browse the repository at this point in the history
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
  • Loading branch information
owenca committed May 9, 2023
1 parent e494ebf commit a4c87f8
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 23 deletions.
32 changes: 11 additions & 21 deletions clang/lib/Format/WhitespaceManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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, "", "",
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down
45 changes: 45 additions & 0 deletions clang/unittests/Format/FormatTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions clang/unittests/Format/FormatTestComments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -4316,7 +4316,7 @@ TEST_F(FormatTestComments, SplitCommentIntroducers) {
)",
format(R"(//
/\
/
/
)",
getLLVMStyleWithColumns(10)));
}
Expand Down

0 comments on commit a4c87f8

Please sign in to comment.