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

[rymiel]

Summary

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

[rymiel]

Note: this patch also makes LevelIndentTracker::skipLine obsolete. I suppose I could clean that up in a following patch

[HazardyKnusperkeks]

There is no PP stuff in the new tests.

And I see the style was bogus before, but shouldn't all variables start with a capital letter? Please fix that in the code you touch.

[owenpan]

In D144296#4136133, @rymiel wrote:

Note: this patch also makes LevelIndentTracker::skipLine obsolete. I suppose I could clean that up in a following patch

I'd delete it in this patch.

clang/lib/Format/UnwrappedLineFormatter.cpp

363–364

365–367

368

372

374

outdentBy?

376

378

Did git-clang-format miss this?

[owenpan]

In D144296#4137508, @HazardyKnusperkeks wrote:

And I see the style was bogus before, but shouldn't all variables start with a capital letter? Please fix that in the code you touch.

+1.

[rymiel]

Apply suggestions and add extra test

clang/lib/Format/UnwrappedLineFormatter.cpp

378

Nope, I forgot to amend the change!

[rymiel]

Inline and remove LevelIndentTracker::skipLine

clang/lib/Format/UnwrappedLineFormatter.cpp

public:
   unsigned getIndent() const { return Indent; }
 
   /// Update the indent state given that \p Line is going to be formatted
   /// next.
   void nextLine(const AnnotatedLine &Line) {
     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 &&
           Line.Type == LT_CommentAbovePPDirective))) {
       unsigned PPIndentWidth =
           (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
       Indent = Line.InMacroBody
                    ? Line.PPLevel * PPIndentWidth +
                          (Line.Level - Line.PPLevel) * Style.IndentWidth
                    : Line.Level * PPIndentWidth;
       Indent += AdditionalIndent;
     } else {
       Indent = getIndent(Line.Level);
     }
     if (static_cast<int>(Indent) + Offset >= 0)
       Indent += Offset;
     if (Line.IsContinuation)
       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
   /// level to the same indent.
   /// Note that \c nextLine must have been called before this method.
   void adjustToUnmodifiedLine(const AnnotatedLine &Line) {
     unsigned LevelIndent = Line.First->OriginalColumn;
     if (static_cast<int>(LevelIndent) - Offset >= 0)
tryFitMultipleLinesInOne(LevelIndentTracker &IndentTracker,
     if (FirstNonComment->is(tok::comment)) {
       FirstNonComment = FirstNonComment->getNextNonComment();
       if (!FirstNonComment)
         return 0;
     }
     // FIXME: There are probably cases where we should use FirstNonComment
     // 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)) {
         int i = 0;
         unsigned openingLine = TheLine->MatchingOpeningBlockLineIndex - 1;
         for (; I + 1 + i != E &&
                nsToken->TokenText ==
                    getMatchingNamespaceTokenText(I[i + 1], AnnotatedLines) &&

clang/unittests/Format/FormatTest.cpp

EXPECT_EQ("namespace out { namespace mid {\n"
             "  } // namespace in\n"
             "  int k;\n"
             "}} // namespace out::mid",
             format("namespace out { namespace mid {\n"
                    "namespace in { int j; } // namespace in\n"
                    "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"
             "}} // namespace out::in",
             format("namespace out {\n"
                    "namespace in {\n"
                    "int i;\n"
                    "} // namespace in\n"