[cfe-commits] [PATCH] Implement AST dumper for Decls
Alexander Kornienko
reviews at llvm-reviews.chandlerc.com
Mon Oct 1 07:45:34 PDT 2012
I think you move in the right direction. Though, I'd recommend replacing your awesome but a bit too bulky unit tests with integration tests, see test/Tooling/clang-check-ast-dump.cpp for an example. These are much easier to write, understand and support. See also http://llvm.org/docs/CommandGuide/FileCheck.html for the description of the FileCheck utility used for integration tests.
And a couple of minor inline comments follow.
================
Comment at: lib/AST/StmtDumper.cpp:47
@@ -70,1 +46,3 @@
+ while (CI)
+ DumpSubTree(*CI++);
}
----------------
How about:
for (Stmt::child_range CI = S->children(); CI; ++CI)
DumpSubTree(*CI);
?
================
Comment at: lib/AST/StmtDumper.cpp:52
@@ -75,10 +51,3 @@
}
- --IndentLevel;
- }
-
- void DumpDeclarator(Decl *D);
-
- void Indent() const {
- for (int i = 0, e = IndentLevel; i < e; ++i)
- OS << " ";
+ Dumper.unindent();
}
----------------
Did you consider replacing indent/unindent pairs with something like:
```class IndentScope {
ASTDumper &Dumper;
public:
IndentScope(ASTDumper &Dumper) : Dumper(Dumper) {
Dumper.indent();
}
~IndentScope() {
Dumper.unindent();
}
};
...
while (...) {
IndentScope Indent(Dumper);
...
}
```
?
http://llvm-reviews.chandlerc.com/D52
More information about the cfe-commits
mailing list