[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