aaron.ballman added inline comments.

================
Comment at: include/clang/AST/ASTDumperUtils.h:22
+
+namespace ast_dumper {
+// Colors used for various parts of the AST dump
----------------
I'm not certain this namespace is useful, especially when it gets imported at 
TU scope in ASTDumper.cpp.


================
Comment at: include/clang/AST/ASTDumperUtils.h:96
+
+struct TextChildDumper {
+  raw_ostream &OS;
----------------
I'm not sold on the name for this class. It's a bit too generic to understand 
what it does. How about `ASTDumpLayoutFormatter` (and `ASTDumpNodeFormatter` 
for the node dumper)?


================
Comment at: include/clang/AST/ASTDumperUtils.h:97-110
+  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.
----------------
I don't think these members should be public.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55188/new/

https://reviews.llvm.org/D55188



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to