[clang] 7ba9101 - [clang-format] Rewrite how indent is reduced for compacted namespaces

[Emilia Dreamer via cfe-commits]

Author: Emilia Dreamer
Date: 2023-02-25T12:13:53+02:00
New Revision: 7ba91016c6ad4cb8f82ed154753ddeeb524f9a64

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

LOG: [clang-format] Rewrite how indent is reduced for compacted namespaces

The previous version set the indentation directly using IndentForLevel,
however, this has a few caveats, namely:

* IndentForLevel applies to all scopes of the entire program being
  formatted, but this indentation should only be adjusted for scopes
  of namespaces.

* The method it used only set the correct indent amount if one wasn't
  already set for a given level, meaning it didn't work correctly if
  anything with indentation preceded a namespace keyword. This
  includes preprocessing directives if using IndentPPDirectives.

This patch instead reduces the Level of all lines within namespaces
which are compacted by CompactNamespaces. This means they will get
correct indentation using the normal process.

Fixes https://github.com/llvm/llvm-project/issues/60843

Reviewed By: owenpan, MyDeveloperDay, HazardyKnusperkeks

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 2e3441e6caec..b0314d6cfa75 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -60,7 +60,8 @@ class LevelIndentTracker {
     Offset = getIndentOffset(*Line.First);
     // Update the indent level cache size so that we can rely on it
     // having the right size in adjustToUnmodifiedline.
-    skipLine(Line, /*UnknownIndent=*/true);
+    if (Line.Level >= IndentForLevel.size())
+      IndentForLevel.resize(Line.Level + 1, -1);
     if (Style.IndentPPDirectives != FormatStyle::PPDIS_None &&
         (Line.InPPDirective ||
          (Style.IndentPPDirectives == FormatStyle::PPDIS_BeforeHash &&
@@ -81,13 +82,6 @@ class LevelIndentTracker {
       Indent = Line.Level * Style.IndentWidth + Style.ContinuationIndentWidth;
   }
 
-  /// Update the indent state given that \p Line indent should be
-  /// skipped.
-  void skipLine(const AnnotatedLine &Line, bool UnknownIndent = false) {
-    if (Line.Level >= IndentForLevel.size())
-      IndentForLevel.resize(Line.Level + 1, UnknownIndent ? -1 : Indent);
-  }
-
   /// Update the level indent to adapt to the given \p Line.
   ///
   /// When a line is not formatted, we move the subsequent lines on the same
@@ -367,20 +361,27 @@ class LineJoiner {
     // instead of TheLine->First.
 
     if (Style.CompactNamespaces) {
-      if (auto nsToken = TheLine->First->getNamespaceToken()) {
-        int i = 0;
-        unsigned closingLine = TheLine->MatchingClosingBlockLineIndex - 1;
-        for (; I + 1 + i != E &&
-               nsToken->TokenText == getNamespaceTokenText(I[i + 1]) &&
-               closingLine == I[i + 1]->MatchingClosingBlockLineIndex &&
-               I[i + 1]->Last->TotalLength < Limit;
-             i++, --closingLine) {
-          // No extra indent for compacted namespaces.
-          IndentTracker.skipLine(*I[i + 1]);
-
-          Limit -= I[i + 1]->Last->TotalLength;
+      if (const auto *NSToken = TheLine->First->getNamespaceToken()) {
+        int J = 1;
+        assert(TheLine->MatchingClosingBlockLineIndex > 0);
+        for (auto ClosingLineIndex = TheLine->MatchingClosingBlockLineIndex - 1;
+             I + J != E && NSToken->TokenText == getNamespaceTokenText(I[J]) &&
+             ClosingLineIndex == I[J]->MatchingClosingBlockLineIndex &&
+             I[J]->Last->TotalLength < Limit;
+             ++J, --ClosingLineIndex) {
+          Limit -= I[J]->Last->TotalLength;
+
+          // Reduce indent level for bodies of namespaces which were compacted,
+          // but only if their content was indented in the first place.
+          auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
+          auto OutdentBy = I[J]->Level - TheLine->Level;
+          for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
+               ++CompactedLine) {
+            if (!(*CompactedLine)->InPPDirective)
+              (*CompactedLine)->Level -= OutdentBy;
+          }
         }
-        return i;
+        return J - 1;
       }
 
       if (auto nsToken = getMatchingNamespaceToken(TheLine, AnnotatedLines)) {

diff  --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 97a0bfae2701..437e7b62a15d 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -4431,6 +4431,46 @@ TEST_F(FormatTest, FormatsCompactNamespaces) {
                    "int k; }} // namespace out::mid",
                    Style));
 
+  verifyFormat("namespace A { namespace B { namespace C {\n"
+               "  int i;\n"
+               "}}} // namespace A::B::C\n"
+               "int main() {\n"
+               "  if (true)\n"
+               "    return 0;\n"
+               "}",
+               "namespace A { namespace B {\n"
+               "namespace C {\n"
+               "  int i;\n"
+               "}} // namespace B::C\n"
+               "} // namespace A\n"
+               "int main() {\n"
+               "  if (true)\n"
+               "    return 0;\n"
+               "}",
+               Style);
+
+  verifyFormat("namespace A { namespace B { namespace C {\n"
+               "#ifdef FOO\n"
+               "  int i;\n"
+               "#endif\n"
+               "}}} // namespace A::B::C\n"
+               "int main() {\n"
+               "  if (true)\n"
+               "    return 0;\n"
+               "}",
+               "namespace A { namespace B {\n"
+               "namespace C {\n"
+               "#ifdef FOO\n"
+               "  int i;\n"
+               "#endif\n"
+               "}} // namespace B::C\n"
+               "} // namespace A\n"
+               "int main() {\n"
+               "  if (true)\n"
+               "    return 0;\n"
+               "}",
+               Style);
+
   Style.NamespaceIndentation = FormatStyle::NI_Inner;
   EXPECT_EQ("namespace out { namespace in {\n"
             "  int i;\n"