[PATCH] Use const visitors in ASTDumper
Alexander Kornienko
alexfh at google.com
Thu Jan 31 07:56:00 PST 2013
Looks good in general.
As for the make_ptr/make_const_ptr - there will be three pairs of them after your change. I'm not sure though, if it makes sense to pull them out to a separate header. I'd also put the DeclVisitorBase class to the same namespace, but I have no strong feelings about that.
There are a few minor formatting comments inline (I hope, some day clang-format will kill the whole idea of discussing formatting in reviews ;).
================
Comment at: lib/AST/ASTDumper.cpp:1543
@@ -1537,1 +1542,3 @@
+void
+ASTDumper::VisitCompoundAssignOperator(const CompoundAssignOperator *Node) {
VisitExpr(Node);
----------------
I'd personally prefer to break the line before "const" instead (it's more common at least in this file).
================
Comment at: lib/AST/ASTDumper.cpp:1509
@@ -1504,1 +1508,3 @@
+void
+ASTDumper::VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *Node) {
VisitExpr(Node);
----------------
I'd personally prefer to break the line before "const" instead (it's more common at least in this file).
================
Comment at: lib/AST/ASTDumper.cpp:1317
@@ -1311,3 +1316,3 @@
VisitStmt(Node);
- for (DeclStmt::decl_iterator I = Node->decl_begin(), E = Node->decl_end();
- I != E; ++I) {
+ for (DeclStmt::const_decl_iterator I = Node->decl_begin(),
+ E = Node->decl_end(); I != E; ++I) {
----------------
How about this version (suggested by clang-format)?
for (DeclStmt::const_decl_iterator I = Node->decl_begin(),
E = Node->decl_end();
I != E; ++I) {
("I = ..." and "E = ..." starting on the same column, "I != E..." aligned as "DeclStmt::...")
================
Comment at: lib/AST/ASTDumper.cpp:777
@@ -778,1 +776,3 @@
+ for (FunctionDecl::param_const_iterator I = D->param_begin(),
+ E = D->param_end(); I != E; ++I) {
if (I + 1 == E)
----------------
I'd indent "E = " to the same column as "I = ", and "I != E; ++I" on the next line as "FunctionDecl::"
================
Comment at: lib/AST/ASTDumper.cpp:1002
@@ -997,2 +1001,3 @@
-void ASTDumper::VisitTemplateTemplateParmDecl(TemplateTemplateParmDecl *D) {
+void
+ASTDumper::VisitTemplateTemplateParmDecl(const TemplateTemplateParmDecl *D) {
----------------
I'd personally prefer to break the line before "const" instead (it's more common at least in this file).
http://llvm-reviews.chandlerc.com/D355
More information about the cfe-commits
mailing list