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