aaron.ballman added inline comments.
================
Comment at: include/clang/AST/ASTDumperUtils.h:115
+ template <typename Fn>
+ void addChild(const std::string &label, Fn doAddChild) {
+ if (label.empty())
----------------
label -> Label
doAddChild -> DoAddChild
================
Comment at: include/clang/AST/ASTDumperUtils.h:117
+ if (label.empty())
+ return addChild(doAddChild);
+ addChild([=] {
----------------
While correct, this is a bit too clever -- can you switch to using a more
idiomatic return statement?
================
Comment at: lib/AST/ASTDumper.cpp:66
+ template <typename Fn>
+ void dumpChild(const std::string &label, Fn doDumpChild) {
+ TreeStructure.addChild(label, doDumpChild);
----------------
Label, DoDumpChild
================
Comment at: lib/AST/ASTDumper.cpp:89
void dumpDecl(const Decl *D);
- void dumpStmt(const Stmt *S);
+ void dumpStmt(const Stmt *S, const std::string &label = {});
----------------
Label
Rather than using `{}`, how about `""` (same behavior, but looks more
idiomatic).
Why `std::string` instead of `StringRef`? I expect this will be called mostly
with string literals, which saves an allocation. The other labels are using
`const char *`, which would be another reasonable option. Whatever we go with,
it'd be nice to make the label types agree across the calls.
================
Comment at: lib/AST/ASTDumper.cpp:1692
-void ASTDumper::dumpStmt(const Stmt *S) {
- dumpChild([=] {
+void ASTDumper::dumpStmt(const Stmt *S, const std::string &label) {
+ dumpChild(label, [=] {
----------------
label -> Label
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55488/new/
https://reviews.llvm.org/D55488
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
