[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