[clang] c077975 - [clang-format] Fix alignment in #else preprocessor blocks

[via cfe-commits]

Author: mitchell
Date: 2022-09-27T15:41:09-04:00
New Revision: c0779756a0c4cc84d9f98714734d47879701cc3d

URL: https://github.com/llvm/llvm-project/commit/c0779756a0c4cc84d9f98714734d47879701cc3d
DIFF: https://github.com/llvm/llvm-project/commit/c0779756a0c4cc84d9f98714734d47879701cc3d.diff

LOG: [clang-format] Fix alignment in #else preprocessor blocks
Summary:
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.

Fixes #36070

Reviewers: HazardyKnusperkeks, MyDeveloperDay

Tags: clang, clang-format

Differential Revision: https://reviews.llvm.org/D134042

Added: 
    

Modified: 
    clang/lib/Format/WhitespaceManager.cpp
    clang/unittests/Format/FormatTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index 6ec788ad23c66..895d0c8e818c2 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -522,6 +522,13 @@ 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.
@@ -556,8 +563,19 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
 
   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;

diff  --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index f27fff1e9ebd0..4c11104343a4d 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -5825,6 +5825,98 @@ TEST_F(FormatTest, IndentPreprocessorDirectives) {
                Style);
 }
 
+TEST_F(FormatTest, FormatAlignInsidePreprocessorElseBlock) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveAssignments.Enabled = true;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
+
+  // Test with just #if blocks.
+  verifyFormat("void f1() {\n"
+               "#if 1\n"
+               "  int foo    = 1;\n"
+               "  int foobar = 2;\n"
+               "#endif\n"
+               "}\n"
+               "#if 1\n"
+               "int baz = 3;\n"
+               "#endif\n"
+               "void f2() {\n"
+               "#if 1\n"
+               "  char *foobarbaz = \"foobarbaz\";\n"
+               "  int   quux      = 4;\n"
+               "}",
+               Style);
+
+  // Test with just #else blocks.
+  verifyFormat("void f1() {\n"
+               "#if 1\n"
+               "#else\n"
+               "  int foo    = 1;\n"
+               "  int foobar = 2;\n"
+               "#endif\n"
+               "}\n"
+               "#if 1\n"
+               "#else\n"
+               "int baz = 3;\n"
+               "#endif\n"
+               "void f2() {\n"
+               "#if 1\n"
+               "#else\n"
+               "  char *foobarbaz = \"foobarbaz\";\n"
+               "  int   quux      = 4;\n"
+               "}",
+               Style);
+
+  // Test with a mix of #if and #else blocks.
+  verifyFormat("void f1() {\n"
+               "#if 1\n"
+               "#else\n"
+               "  int foo    = 1;\n"
+               "  int foobar = 2;\n"
+               "#endif\n"
+               "}\n"
+               "#if 1\n"
+               "int baz = 3;\n"
+               "#endif\n"
+               "void f2() {\n"
+               "#if 1\n"
+               "#else\n"
+               "  // prevent alignment with #else in f1\n"
+               "  char *foobarbaz = \"foobarbaz\";\n"
+               "  int   quux      = 4;\n"
+               "}",
+               Style);
+
+  // Test with nested #if and #else blocks.
+  verifyFormat("void f1() {\n"
+               "#if 1\n"
+               "#else\n"
+               "#if 2\n"
+               "#else\n"
+               "  int foo    = 1;\n"
+               "  int foobar = 2;\n"
+               "#endif\n"
+               "#endif\n"
+               "}\n"
+               "#if 1\n"
+               "#else\n"
+               "#if 2\n"
+               "int baz = 3;\n"
+               "#endif\n"
+               "#endif\n"
+               "void f2() {\n"
+               "#if 1\n"
+               "#if 2\n"
+               "#else\n"
+               "  // prevent alignment with #else in f1\n"
+               "  char *foobarbaz = \"foobarbaz\";\n"
+               "  int   quux      = 4;\n"
+               "#endif\n"
+               "#endif\n"
+               "}",
+               Style);
+}
+
 TEST_F(FormatTest, FormatHashIfNotAtStartOfLine) {
   verifyFormat("{\n  { a #c; }\n}");
 }