[clang] 0570cc5 - [clang-format] Fix indent for selective formatting

[Owen Pan via cfe-commits]

Author: Sedenion
Date: 2023-07-18T14:10:53-07:00
New Revision: 0570cc568f5c8047267be7ab5056f8c4f45349b5

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

LOG: [clang-format] Fix indent for selective formatting

The problem was that the LevelIndentTracker remembered
the indentation level of previous deeper levels when
leaving a scope. Afterwards, when it entered again a
deeper level, it blindly reused the previous
indentation level. In case the --lines option was used
such that the previous deeper level was not formatted,
that previous level was whatever happened to be there
in the source code. The formatter simply believed it.

This is fixed by letting the LevelIndentTracker forget
the previous deeper levels when stepping out of them
(=> change in LevelIndentTracker::nextLine()).
Note that this used to be the case until LLVM 14.0.6,
but was changed in https://reviews.llvm.org/D129064
(#56352) to fix a crash. Our commit here essentially
reverts that crash fix. It was incorrect/incomplete.

Fixes #58464.
Fixes #59178.
Fixes #62799.

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

Added: 
    

Modified: 
    clang/lib/Format/UnwrappedLineFormatter.cpp
    clang/unittests/Format/FormatTestSelective.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index b745838c1cc5e4..52519145279cce 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -74,6 +74,13 @@ class LevelIndentTracker {
                    : Line.Level * PPIndentWidth;
       Indent += AdditionalIndent;
     } else {
+      // When going to lower levels, forget previous higher levels so that we
+      // recompute future higher levels. But don't forget them if we enter a PP
+      // directive, since these do not terminate a C++ code block.
+      if (!Line.InPPDirective) {
+        assert(Line.Level <= IndentForLevel.size());
+        IndentForLevel.resize(Line.Level + 1);
+      }
       Indent = getIndent(Line.Level);
     }
     if (static_cast<int>(Indent) + Offset >= 0)

diff  --git a/clang/unittests/Format/FormatTestSelective.cpp b/clang/unittests/Format/FormatTestSelective.cpp
index 86ed7aba1913d4..da252eb2ae3c38 100644
--- a/clang/unittests/Format/FormatTestSelective.cpp
+++ b/clang/unittests/Format/FormatTestSelective.cpp
@@ -528,6 +528,26 @@ TEST_F(FormatTestSelective, ReformatRegionAdjustsIndent) {
             format("  int a;\n"
                    "void ffffff() {}",
                    11, 0));
+
+  // https://github.com/llvm/llvm-project/issues/59178
+  Style = getMozillaStyle();
+  EXPECT_EQ("int a()\n"
+            "{\n"
+            "return 0;\n"
+            "}\n"
+            "int b()\n"
+            "{\n"
+            "  return 42;\n"
+            "}",
+            format("int a()\n"
+                   "{\n"
+                   "return 0;\n"
+                   "}\n"
+                   "int b()\n"
+                   "{\n"
+                   "return 42;\n" // Format this line only
+                   "}",
+                   32, 0));
 }
 
 TEST_F(FormatTestSelective, UnderstandsTabs) {
@@ -617,6 +637,61 @@ TEST_F(FormatTestSelective, DontAssert) {
          "namespace ns1 { namespace ns2 {\n"
          "}}";
   EXPECT_EQ(Code, format(Code, 0, 0));
+
+  // https://reviews.llvm.org/D151047#4369742
+  Style = getLLVMStyle();
+  Style.FixNamespaceComments = false;
+  Code = "namespace ns {\n"
+         "#define REF(alias) alias alias_var;\n"
+         "}"; // Format this line only
+  EXPECT_EQ(Code, format(Code, 51, 0));
+}
+
+TEST_F(FormatTestSelective, FormatMacroRegardlessOfPreviousIndent) {
+  // clang-format currently does not (or should not) take into account the
+  // indent of previous unformatted lines when formatting a PP directive.
+  // Technically speaking, LevelIndentTracker::IndentForLevel is only for non-PP
+  // lines. So these tests here check that the indent of previous non-PP lines
+  // do not affect the formatting. If this requirement changes, the tests here
+  // need to be adapted.
+  Style = getLLVMStyle();
+
+  const StringRef Code{"      class Foo {\n"
+                       "            void test() {\n"
+                       "    #ifdef 1\n"
+                       "                #define some\n" // format this line
+                       "         #endif\n"
+                       "    }};"};
+
+  EXPECT_EQ(Style.IndentPPDirectives,
+            FormatStyle::PPDirectiveIndentStyle::PPDIS_None);
+  EXPECT_EQ("      class Foo {\n"
+            "            void test() {\n"
+            "    #ifdef 1\n"
+            "#define some\n" // Formatted line
+            "#endif\n"       // That this line is also formatted might be a bug.
+            "            }};", // Ditto: Bug?
+            format(Code, 57, 0));
+
+  Style.IndentPPDirectives =
+      FormatStyle::PPDirectiveIndentStyle::PPDIS_BeforeHash;
+  EXPECT_EQ("      class Foo {\n"
+            "            void test() {\n"
+            "    #ifdef 1\n"
+            "  #define some\n" // Formatted line
+            "         #endif\n"
+            "    }};",
+            format(Code, 57, 0));
+
+  Style.IndentPPDirectives =
+      FormatStyle::PPDirectiveIndentStyle::PPDIS_AfterHash;
+  EXPECT_EQ("      class Foo {\n"
+            "            void test() {\n"
+            "    #ifdef 1\n"
+            "#  define some\n" // Formatted line
+            "#endif\n" // That this line is also formatted might be a bug.
+            "    }};",
+            format(Code, 57, 0));
 }
 
 } // end namespace