From a4c87f8ccaccc76fd7d1c6c2e639ca84b9ec7794 Mon Sep 17 00:00:00 2001 From: Owen Pan Date: Sat, 6 May 2023 16:29:16 -0700 Subject: [PATCH] [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 --- clang/lib/Format/WhitespaceManager.cpp | 32 +++++++------------ clang/unittests/Format/FormatTest.cpp | 45 +++++++++++++++++++++++++++ clang/unittests/Format/FormatTestComments.cpp | 4 +-- 3 files changed, 58 insertions(+), 23 deletions(-) diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index d2be3b3..040b953 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(); - // 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(); - // 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 099a5cb..dc67393 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 60e0450..6e5b5b2 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))); } -- 2.7.4