[cfe-commits] [PATCH] Implement AST dumper for Decls

[Philip Craig]

  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