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

[Alexander Kornienko]

  Except a couple of things (see inline comments) this looks good to me. I'm not sure it makes sense to split this patch, but others may have different opinions.

  FYI: I'm on vacation now, so I'll be very slow to respond in the next 3 weeks.

  Doug, can you look at this?


================
Comment at: lib/AST/ASTDumper.cpp:44
@@ +43,3 @@
+void ASTDumper::dumpAttrs(AttrVec &Attrs) {
+  for (AttrVec::const_iterator I=Attrs.begin(), E=Attrs.end(); I != E; ++I) {
+    IndentScope Indent(*this);
----------------
Please add spaces around '='

================
Comment at: lib/AST/ASTDumper.cpp:52
@@ +51,3 @@
+    }
+    OS << "Attr" << " " << (const void*)A;
+    dumpSourceRange(A->getRange());
----------------
Why are "Attr" and " " printed separately?

================
Comment at: lib/AST/ASTDumper.cpp:32
@@ +31,3 @@
+  NeedNewLine = true;
+  for (int i = 0, e = IndentLevel; i < e; ++i)
+    OS << "  ";
----------------
I think, it'd be better "OS.indent(IndentLevel * 2);" (http://llvm.org/docs/doxygen/html/classllvm_1_1raw__ostream.html#a8fdf5cdf041c8aded7e3308c1c3efacc)

================
Comment at: test/Misc/ast-dump-decl.c:10
@@ +9,3 @@
+// CHECK:      {{^}}(RecordDecl{{.*}}TestIndent{{[^()]*$}}
+// CHECK-NEXT: {{^  }}(FieldDecl{{.*}}x{{[^()]*}})){{$}}
+
----------------
This is a matter of taste, probably, but I'd prefer to see one regex instead of 3 and more in a line, this also can make indentation a bit clearer. Just for comparison:
{{^\(RecordDecl.*TestIndent[^()]*$}}
{{^  \(FieldDecl.*x[^()]*\)\)$}}


http://llvm-reviews.chandlerc.com/D52