Which part of the unit tests do you think is bulky? The initial
infrastructure is mostly a copy from StmtPrinterTest.cpp. This shouldn't need
to be changed when adding tests (and probably should be moved to some common
code), so the size of the actual tests is similar to what is needed for
FileCheck.
I used FileCheck for the tests for my standalone dumper, and had a couple of
problems. It:
* uses the same compiler options and language for the whole test
* currently ignores leading whitespace so can't test indentation
* is fragile if you want to test the SourceRange dumping
The ability to use an AST matcher for unit tests also makes them slightly
more convenient (I realise there'll be a command line option for this
eventually).
The main advantage I can see for FileCheck is that it provides more flexible
matching.
================
Comment at: lib/AST/StmtDumper.cpp:47
@@ -70,1 +46,3 @@
+ while (CI)
+ DumpSubTree(*CI++);
}
----------------
Alexander Kornienko wrote:
> How about:
> for (Stmt::child_range CI = S->children(); CI; ++CI)
> DumpSubTree(*CI);
> ?
Fixed.
================
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();
}
----------------
Alexander Kornienko wrote:
> 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);
> ...
> }
> ```
> ?
I didn't, but I like that; done. I also added a flush() to ~ASTDumper().
http://llvm-reviews.chandlerc.com/D52
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits