Implement the TreeStructure interface through the TextNodeDumper

[steveire]

Summary

This way, when the generic ASTTraverser is extracted from ASTDumper,
there can't be any problem related to ordering of class members, a
concern that was raised in https://reviews.llvm.org/D55337.

This will also preserve the property that the generic traverser does not
enforce any coupling between the NodeDumper and the TreeStructure.

https://godbolt.org/z/PEtT1_

[aaron.ballman]

I think this is a good approach to solving the problem, so let's go this route.

[erichkeane]

In D56407#1349672, @aaron.ballman wrote:

I think this is a good approach to solving the problem, so let's go this route.

Yeah, I agree here. This ends up being a really clean solution.

include/clang/AST/ASTDumperUtils.h

if (ShowColors)
       OS.changeColor(Color.Color, Color.Bold);
   }
   ~ColorScope() {
     if (ShowColors)
       OS.resetColor();
   }
 };
 
-class TextTreeStructure {
-  raw_ostream &OS;
-  const bool ShowColors;
-
-  /// Pending[i] is an action to dump an entity at level i.
-  llvm::SmallVector<std::function<void(bool isLastChild)>, 32> Pending;
-
-  /// Indicates whether we're at the top level.
-  bool TopLevel = true;
-
-  /// Indicates if we're handling the first child after entering a new depth.
-  bool FirstChild = true;
-
-  /// Prefix for currently-being-dumped entity.
-  std::string Prefix;
-
-public:
-  /// Add a child of the current node.  Calls doAddChild without arguments
-  template <typename Fn> void addChild(Fn doAddChild) {
-    // If we're at the top level, there's nothing interesting to do; just
-    // run the dumper.
-    if (TopLevel) {
-      TopLevel = false;
-      doAddChild();
-      while (!Pending.empty()) {
-        Pending.back()(true);
-        Pending.pop_back();
-      }
-      Prefix.clear();
-      OS << "\n";
-      TopLevel = true;
-      return;
-    }
-
-    auto dumpWithIndent = [this, doAddChild](bool isLastChild) {
-      // Print out the appropriate tree structure and work out the prefix for
-      // children of this node. For instance:
-      //
-      //   A        Prefix = ""
-      //   |-B      Prefix = "| "
-      //   | `-C    Prefix = "|   "
-      //   `-D      Prefix = "  "
-      //     |-E    Prefix = "  | "
-      //     `-F    Prefix = "    "
-      //   G        Prefix = ""
-      //
-      // Note that the first level gets no prefix.
-      {
-        OS << '\n';
-        ColorScope Color(OS, ShowColors, IndentColor);
-        OS << Prefix << (isLastChild ? '`' : '|') << '-';
-        this->Prefix.push_back(isLastChild ? ' ' : '|');
-        this->Prefix.push_back(' ');
-      }
-
-      FirstChild = true;
-      unsigned Depth = Pending.size();
-
-      doAddChild();
-
-      // If any children are left, they're the last at their nesting level.
-      // Dump those ones out now.
-      while (Depth < Pending.size()) {
-        Pending.back()(true);
-        this->Pending.pop_back();
-      }
-
-      // Restore the old prefix.
-      this->Prefix.resize(Prefix.size() - 2);
-    };
-
-    if (FirstChild) {
-      Pending.push_back(std::move(dumpWithIndent));
-    } else {
-      Pending.back()(false);
-      Pending.back() = std::move(dumpWithIndent);
-    }
-    FirstChild = false;
-  }
-
-  TextTreeStructure(raw_ostream &OS, bool ShowColors)
-      : OS(OS), ShowColors(ShowColors) {}
-};
-
 } // namespace clang
 
 #endif // LLVM_CLANG_AST_ASTDUMPERUTILS_H

include/clang/AST/TextNodeDumper.h

 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDumperUtils.h"
 #include "clang/AST/CommentCommandTraits.h"
 #include "clang/AST/CommentVisitor.h"
 #include "clang/AST/ExprCXX.h"
 
 namespace clang {
 
+class TextTreeStructure {
+  raw_ostream &OS;
+  const bool ShowColors;
+
+  /// Pending[i] is an action to dump an entity at level i.
+  llvm::SmallVector<std::function<void(bool isLastChild)>, 32> Pending;
+
+  /// Indicates whether we're at the top level.
+  bool TopLevel = true;
+
+  /// Indicates if we're handling the first child after entering a new depth.
+  bool FirstChild = true;
+
+  /// Prefix for currently-being-dumped entity.
+  std::string Prefix;
+
+public:
+  /// Add a child of the current node.  Calls doAddChild without arguments
+  template <typename Fn> void addChild(Fn doAddChild) {
+    // If we're at the top level, there's nothing interesting to do; just
+    // run the dumper.
+    if (TopLevel) {
+      TopLevel = false;
+      doAddChild();
+      while (!Pending.empty()) {
+        Pending.back()(true);
+        Pending.pop_back();
+      }
+      Prefix.clear();
+      OS << "\n";
+      TopLevel = true;
+      return;
+    }
+
+    auto dumpWithIndent = [this, doAddChild](bool isLastChild) {
+      // Print out the appropriate tree structure and work out the prefix for
+      // children of this node. For instance:
+      //
+      //   A        Prefix = ""
+      //   |-B      Prefix = "| "
+      //   | `-C    Prefix = "|   "
+      //   `-D      Prefix = "  "
+      //     |-E    Prefix = "  | "
+      //     `-F    Prefix = "    "
+      //   G        Prefix = ""
+      //
+      // Note that the first level gets no prefix.
+      {
+        OS << '\n';
+        ColorScope Color(OS, ShowColors, IndentColor);
+        OS << Prefix << (isLastChild ? '`' : '|') << '-';
+        this->Prefix.push_back(isLastChild ? ' ' : '|');
+        this->Prefix.push_back(' ');
+      }
+
+      FirstChild = true;
+      unsigned Depth = Pending.size();
+
+      doAddChild();
+
+      // If any children are left, they're the last at their nesting level.
+      // Dump those ones out now.
+      while (Depth < Pending.size()) {
+        Pending.back()(true);
+        this->Pending.pop_back();
+      }
+
+      // Restore the old prefix.
+      this->Prefix.resize(Prefix.size() - 2);
+    };
+
+    if (FirstChild) {
+      Pending.push_back(std::move(dumpWithIndent));
+    } else {
+      Pending.back()(false);
+      Pending.back() = std::move(dumpWithIndent);
+    }
+    FirstChild = false;
+  }
+
+  TextTreeStructure(raw_ostream &OS, bool ShowColors)
+      : OS(OS), ShowColors(ShowColors) {}
+};
+
 class TextNodeDumper
     : public comments::ConstCommentVisitor<TextNodeDumper, void,
-                                           const comments::FullComment *> {
+                                           const comments::FullComment *>,
+      public TextTreeStructure {
   raw_ostream &OS;
   const bool ShowColors;
 
   /// Keep track of the last location we print out so that we can
   /// print out deltas from then on out.
   const char *LastLocFilename = "";
   unsigned LastLocLine = ~0U;
 

lib/AST/ASTDumper.cpp

 namespace  {
 
   class ASTDumper
       : public ConstDeclVisitor<ASTDumper>,
         public ConstStmtVisitor<ASTDumper>,
         public ConstCommentVisitor<ASTDumper, void, const FullComment *>,
         public TypeVisitor<ASTDumper> {
 
-    TextTreeStructure TreeStructure;
     TextNodeDumper NodeDumper;
 
     raw_ostream &OS;
 
     /// The policy to use for printing; can be defaulted.
     PrintingPolicy PrintPolicy;
 
     /// Indicates whether we should trigger deserialization of nodes that had
     /// not already been loaded.
     bool Deserialize = false;
 
     const bool ShowColors;
 
     /// Dump a child of the current node.
     template<typename Fn> void dumpChild(Fn doDumpChild) {
-      TreeStructure.addChild(doDumpChild);
+      NodeDumper.addChild(doDumpChild);
     }
 
   public:
     ASTDumper(raw_ostream &OS, const CommandTraits *Traits,
               const SourceManager *SM)
         : ASTDumper(OS, Traits, SM,
                     SM && SM->getDiagnostics().getShowColors()) {}
 
     ASTDumper(raw_ostream &OS, const CommandTraits *Traits,
               const SourceManager *SM, bool ShowColors)
         : ASTDumper(OS, Traits, SM, ShowColors, LangOptions()) {}
     ASTDumper(raw_ostream &OS, const CommandTraits *Traits,
               const SourceManager *SM, bool ShowColors,
               const PrintingPolicy &PrintPolicy)
-        : TreeStructure(OS, ShowColors),
-          NodeDumper(OS, ShowColors, SM, PrintPolicy, Traits), OS(OS),
+        : NodeDumper(OS, ShowColors, SM, PrintPolicy, Traits), OS(OS),
           PrintPolicy(PrintPolicy), ShowColors(ShowColors) {}
 
     void setDeserialize(bool D) { Deserialize = D; }
 
     void dumpDecl(const Decl *D);
     void dumpStmt(const Stmt *S);
 
     // Utilities

lib/AST/TextNodeDumper.cpp

 #include "clang/AST/TextNodeDumper.h"
 
 using namespace clang;
 
 TextNodeDumper::TextNodeDumper(raw_ostream &OS, bool ShowColors,
                                const SourceManager *SM,
                                const PrintingPolicy &PrintPolicy,
                                const comments::CommandTraits *Traits)
-    : OS(OS), ShowColors(ShowColors), SM(SM), PrintPolicy(PrintPolicy),
-      Traits(Traits) {}
+    : TextTreeStructure(OS, ShowColors), OS(OS), ShowColors(ShowColors), SM(SM),
+      PrintPolicy(PrintPolicy), Traits(Traits) {}
 
 void TextNodeDumper::Visit(const comments::Comment *C,
                            const comments::FullComment *FC) {
   if (!C) {
     ColorScope Color(OS, ShowColors, NullColor);
     OS << "<<<NULL>>>";
     return;
   }