[PATCH] D57472: [AST] Extract ASTDumpTraverser class from ASTDumper

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 31 12:17:23 PST 2019


aaron.ballman added inline comments.


================
Comment at: include/clang/AST/ASTDumpTraverser.h:31
+
+ASTDumpTraverser traverses the Clang AST for dumping purposes
+
----------------
Missing full stop at the end of the sentence.


================
Comment at: include/clang/AST/ASTDumpTraverser.h:38
+    struct
+    {
+      template <typename Fn>
----------------
Format this with the usual LLVM style.

Also, I'd remove a bit of the vertical whitespace between the `Visit()` functions.


================
Comment at: include/clang/AST/ASTDumpTraverser.h:83
+
+  NodeVisitorType &getNodeVisitor() { return getDerived().doGetNodeVisitor(); }
+  Derived &getDerived() { return *static_cast<Derived *>(this); }
----------------
Given that `ASTDumpTraverser` is itself a visitor of nodes, I wonder if a better name for this would be `getNodeDumper()` or something (and similarly renaming the template parameter)?


================
Comment at: include/clang/AST/ASTDumpTraverser.h:91-92
+  void Visit(const Decl *D) {
+    getNodeVisitor().AddChild([=] {
+      getNodeVisitor().Visit(D);
+      if (!D)
----------------
I wonder how often we will spot calls to `Visit()` that were meant to go to `getNodeVisitor().Visit()`? Probably not too many because the output would be wrong except if you got unlucky, but I do wonder if the naming similarities will cause confusion as people do new development.

Not certain there's anything to be changed here, but wanted to raise the point in case it sparked discussion.


================
Comment at: lib/AST/ASTDumper.cpp:63
 
-      if (const auto *C = dyn_cast<CXXConstructorDecl>(D))
-        for (const auto *I : C->inits())
-          Visit(I);
-
-      if (D->doesThisDeclarationHaveABody())
-        Visit(D->getBody());
-    }
-
-    void VisitFieldDecl(const FieldDecl *D) {
-      if (D->isBitField())
-        Visit(D->getBitWidth());
-      if (Expr *Init = D->getInClassInitializer())
-        Visit(Init);
-    }
-
-    void VisitVarDecl(const VarDecl *D) {
-      if (D->hasInit())
-        Visit(D->getInit());
-    }
-
-    void VisitDecompositionDecl(const DecompositionDecl *D) {
-      VisitVarDecl(D);
-      for (const auto *B : D->bindings())
-        Visit(B);
-    }
-
-    void VisitBindingDecl(const BindingDecl *D) {
-      if (const auto *E = D->getBinding())
-        Visit(E);
-    }
-
-    void VisitFileScopeAsmDecl(const FileScopeAsmDecl *D) {
-      Visit(D->getAsmString());
-    }
-
-    void VisitCapturedDecl(const CapturedDecl *D) { Visit(D->getBody()); }
-
-    void VisitOMPThreadPrivateDecl(const OMPThreadPrivateDecl *D) {
-      for (const auto *E : D->varlists())
-        Visit(E);
-    }
-
-    void VisitOMPDeclareReductionDecl(const OMPDeclareReductionDecl *D) {
-      Visit(D->getCombiner());
-      if (const auto *Initializer = D->getInitializer())
-        Visit(Initializer);
-    }
-
-    void VisitOMPCapturedExprDecl(const OMPCapturedExprDecl *D) {
-      Visit(D->getInit());
-    }
-
-    template <typename SpecializationDecl>
-    void dumpTemplateDeclSpecialization(const SpecializationDecl *D,
-                                        bool DumpExplicitInst,
-                                        bool DumpRefOnly);
-    template <typename TemplateDecl>
-    void dumpTemplateDecl(const TemplateDecl *D, bool DumpExplicitInst);
-
-    void VisitTypeAliasDecl(const TypeAliasDecl *D) {
-      Visit(D->getUnderlyingType());
-    }
-
-    void VisitTypeAliasTemplateDecl(const TypeAliasTemplateDecl *D) {
-      dumpTemplateParameters(D->getTemplateParameters());
-      Visit(D->getTemplatedDecl());
-    }
-
-    void VisitStaticAssertDecl(const StaticAssertDecl *D) {
-      Visit(D->getAssertExpr());
-      Visit(D->getMessage());
-    }
-    void VisitFunctionTemplateDecl(const FunctionTemplateDecl *D);
-    void VisitClassTemplateDecl(const ClassTemplateDecl *D);
-
-    void VisitClassTemplateSpecializationDecl(
-        const ClassTemplateSpecializationDecl *D) {
-      dumpTemplateArgumentList(D->getTemplateArgs());
-    }
-
-    void VisitClassTemplatePartialSpecializationDecl(
-        const ClassTemplatePartialSpecializationDecl *D) {
-      VisitClassTemplateSpecializationDecl(D);
-      dumpTemplateParameters(D->getTemplateParameters());
-    }
-
-    void VisitClassScopeFunctionSpecializationDecl(
-        const ClassScopeFunctionSpecializationDecl *D) {
-      Visit(D->getSpecialization());
-      if (D->hasExplicitTemplateArgs())
-        dumpTemplateArgumentListInfo(D->templateArgs());
-    }
-    void VisitVarTemplateDecl(const VarTemplateDecl *D);
-
-    void VisitBuiltinTemplateDecl(const BuiltinTemplateDecl *D) {
-      dumpTemplateParameters(D->getTemplateParameters());
-    }
-
-    void
-    VisitVarTemplateSpecializationDecl(const VarTemplateSpecializationDecl *D) {
-      dumpTemplateArgumentList(D->getTemplateArgs());
-      VisitVarDecl(D);
-    }
-
-    void VisitVarTemplatePartialSpecializationDecl(
-        const VarTemplatePartialSpecializationDecl *D) {
-      dumpTemplateParameters(D->getTemplateParameters());
-      VisitVarTemplateSpecializationDecl(D);
-    }
-
-    void VisitTemplateTypeParmDecl(const TemplateTypeParmDecl *D) {
-      if (D->hasDefaultArgument())
-        Visit(D->getDefaultArgument(), SourceRange(),
-              D->getDefaultArgStorage().getInheritedFrom(),
-              D->defaultArgumentWasInherited() ? "inherited from" : "previous");
-    }
-
-    void VisitNonTypeTemplateParmDecl(const NonTypeTemplateParmDecl *D) {
-      if (D->hasDefaultArgument())
-        Visit(D->getDefaultArgument(), SourceRange(),
-              D->getDefaultArgStorage().getInheritedFrom(),
-              D->defaultArgumentWasInherited() ? "inherited from" : "previous");
-    }
-
-    void VisitTemplateTemplateParmDecl(const TemplateTemplateParmDecl *D) {
-      dumpTemplateParameters(D->getTemplateParameters());
-      if (D->hasDefaultArgument())
-        dumpTemplateArgumentLoc(
-            D->getDefaultArgument(),
-            D->getDefaultArgStorage().getInheritedFrom(),
-            D->defaultArgumentWasInherited() ? "inherited from" : "previous");
-    }
-
-    void VisitUsingShadowDecl(const UsingShadowDecl *D) {
-      if (auto *TD = dyn_cast<TypeDecl>(D->getUnderlyingDecl()))
-        Visit(TD->getTypeForDecl());
-    }
-
-    void VisitFriendDecl(const FriendDecl *D) {
-      if (!D->getFriendType())
-        Visit(D->getFriendDecl());
-    }
-
-    void VisitObjCMethodDecl(const ObjCMethodDecl *D) {
-      if (D->isThisDeclarationADefinition())
-        dumpDeclContext(D);
-      else
-        for (const ParmVarDecl *Parameter : D->parameters())
-          Visit(Parameter);
-
-      if (D->hasBody())
-        Visit(D->getBody());
-    }
-
-    void VisitObjCCategoryDecl(const ObjCCategoryDecl *D) {
-      dumpObjCTypeParamList(D->getTypeParamList());
-    }
-
-    void VisitObjCInterfaceDecl(const ObjCInterfaceDecl *D) {
-      dumpObjCTypeParamList(D->getTypeParamListAsWritten());
-    }
-
-    void VisitObjCImplementationDecl(const ObjCImplementationDecl *D) {
-      for (const auto &I : D->inits())
-        Visit(I);
-    }
-
-    void VisitBlockDecl(const BlockDecl *D) {
-      for (const auto &I : D->parameters())
-        Visit(I);
-
-      for (const auto &I : D->captures())
-        Visit(I);
-      Visit(D->getBody());
-    }
-
-    void VisitDeclStmt(const DeclStmt *Node) {
-      for (const auto &D : Node->decls())
-        Visit(D);
-    }
-
-    void VisitAttributedStmt(const AttributedStmt *Node) {
-      for (const auto *A : Node->getAttrs())
-        Visit(A);
-    }
-
-    void VisitCXXCatchStmt(const CXXCatchStmt *Node) {
-      Visit(Node->getExceptionDecl());
-    }
-
-    void VisitCapturedStmt(const CapturedStmt *Node) {
-      Visit(Node->getCapturedDecl());
-    }
-
-    void VisitOMPExecutableDirective(const OMPExecutableDirective *Node) {
-      for (const auto *C : Node->clauses())
-        Visit(C);
-    }
-
-    void VisitInitListExpr(const InitListExpr *ILE) {
-      if (auto *Filler = ILE->getArrayFiller()) {
-        Visit(Filler, "array_filler");
-      }
-    }
-
-    void VisitBlockExpr(const BlockExpr *Node) { Visit(Node->getBlockDecl()); }
-
-    void VisitOpaqueValueExpr(const OpaqueValueExpr *Node) {
-      if (Expr *Source = Node->getSourceExpr())
-        Visit(Source);
-    }
-
-    void VisitGenericSelectionExpr(const GenericSelectionExpr *E) {
-      Visit(E->getControllingExpr());
-      Visit(E->getControllingExpr()->getType()); // FIXME: remove
-
-      for (const auto &Assoc : E->associations()) {
-        Visit(Assoc);
-      }
-    }
-
-    void VisitLambdaExpr(const LambdaExpr *Node) {
-      Visit(Node->getLambdaClass());
-    }
-
-    void VisitSizeOfPackExpr(const SizeOfPackExpr *Node) {
-      if (Node->isPartiallySubstituted())
-        for (const auto &A : Node->getPartialArguments())
-          Visit(A);
-    }
-
-    void VisitObjCAtCatchStmt(const ObjCAtCatchStmt *Node) {
-      if (const VarDecl *CatchParam = Node->getCatchParamDecl())
-        Visit(CatchParam);
-    }
-
-    void VisitExpressionTemplateArgument(const TemplateArgument &TA) {
-      Visit(TA.getAsExpr());
-    }
-    void VisitPackTemplateArgument(const TemplateArgument &TA) {
-      for (const auto &TArg : TA.pack_elements())
-        Visit(TArg);
-    }
-
-    // Implements Visit methods for Attrs.
-#include "clang/AST/AttrNodeTraverse.inc"
-  };
+  void VisitFunctionTemplateDecl(const FunctionTemplateDecl *D);
+  void VisitClassTemplateDecl(const ClassTemplateDecl *D);
----------------
steveire wrote:
> These visitors are left behind here because how their specializations are handled is quite bespoke in AST-dumping terms.
> 
> The way specializations are dumped could probably be cleaned up a bit to make it possible to move these methods too, but it is not clear what would be acceptable there.
That seems reasonable to me; we can address them in a later patch.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57472/new/

https://reviews.llvm.org/D57472





More information about the cfe-commits mailing list