Implement TemplateArgument dumping in terms of Visitor

[steveire]

Summary

Split the output streaming from the traversal to other AST nodes.

[steveire]

Update

[steveire]

Rebase

[aaron.ballman]

include/clang/AST/TemplateArgumentVisitor.h

24

std::add_lvalue_reference<> already does this, so do we really need this?

25

I'm guessing we can't use std::add_lvalue_reference<std::add_const<>> for the same reasons we couldn't use it for pointers, but then make_const_ref should live in STLExtras.h and not here (it's generally useful).

28–29

s/class/typename to be self-consistent

56

Might be worth it to replace these with a macro, sort of like how DISPATCH is used above.

include/clang/AST/TextNodeDumper.h

71

label -> Label and I think we should use a StringRef for the type.

lib/AST/ASTDumper.cpp

337–338

llvm::for_each(TA.pack_elements(), ...); (or a range-based for loop)

lib/AST/TextNodeDumper.cpp

165

label -> Label

170

Elide braces

[steveire]

lib/AST/ASTDumper.cpp

337–338

I'm just moving the code as it is in its origin location. You could rewrite it separately if you wish.

[aaron.ballman]

lib/AST/ASTDumper.cpp

337–338

I realize that. You're updating existing code, which is a perfectly appropriate time for NFC improvements to it -- we make these kinds of requests frequently because it's the time at which we notice these minor deficiencies.

[steveire]

Updates

[aaron.ballman]

Aside from nits that aren't complete yet, LGTM.

include/clang/AST/TemplateArgumentVisitor.h

25

Missed a typename in there.

[thakis]

Out of interest, what's the motivation for this? It seems to add way more code than it removes, so there must be some other advantage, but the patch description doesn't say.

[steveire]

In D55491#1355105, @thakis wrote:

Out of interest, what's the motivation for this? It seems to add way more code than it removes, so there must be some other advantage, but the patch description doesn't say.

The reason for this is to split the output streaming from the traversal to other AST nodes.

This is a step as part of a larger refactoring with the aim of a class extracted from ASTDumper which does traversal but no streaming. That way, we can dump other output formats and do other things with the traversal.

[steveire]

include/clang/AST/TemplateArgumentVisitor.h

25

If you're talking about the TTP, class must be used until C++17. See https://en.cppreference.com/w/cpp/language/template_parameters

include/clang/AST/TemplateArgumentVisitor.h

+//===- TemplateArgumentVisitor.h - Visitor for TArg subclasses --*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines the TemplateArgumentVisitor interface.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_AST_TEMPLATEARGUMENTVISITOR_H
+#define LLVM_CLANG_AST_TEMPLATEARGUMENTVISITOR_H
+
+#include "clang/AST/TemplateBase.h"
+
+namespace clang {
+
+namespace templateargumentvisitor {
+
+template <typename T> struct make_ref { using type = T &; };
+template <typename T> struct make_const_ref { using type = const T &; };
+
+/// A simple visitor class that helps create template argument visitors.
+template <template <typename> class Ref, typename ImplClass,
+          typename RetTy = void, class... ParamTys>
+class Base {
+public:
+#define REF(CLASS) typename Ref<CLASS>::type
+#define DISPATCH(NAME)                                                         \
+  case TemplateArgument::NAME:                                                 \
+    return static_cast<ImplClass *>(this)->Visit##NAME##TemplateArgument(      \
+        TA, std::forward<ParamTys>(P)...)
+
+  RetTy Visit(REF(TemplateArgument) TA, ParamTys... P) {
+    switch (TA.getKind()) {
+      DISPATCH(Null);
+      DISPATCH(Type);
+      DISPATCH(Declaration);
+      DISPATCH(NullPtr);
+      DISPATCH(Integral);
+      DISPATCH(Template);
+      DISPATCH(TemplateExpansion);
+      DISPATCH(Expression);
+      DISPATCH(Pack);
+    }
+    llvm_unreachable("TemplateArgument is not covered in switch!");
+  }
+
+  // If the implementation chooses not to implement a certain visit
+  // method, fall back to the parent.
+
+  RetTy VisitNullTemplateArgument(REF(TemplateArgument) TA, ParamTys... P) {
+    return VisitTemplateArgument(TA, std::forward<ParamTys>(P)...);
+  }
+  RetTy VisitTypeTemplateArgument(REF(TemplateArgument) TA, ParamTys... P) {
+    return VisitTemplateArgument(TA, std::forward<ParamTys>(P)...);
+  }
+  RetTy VisitDeclarationTemplateArgument(REF(TemplateArgument) TA,
+                                         ParamTys... P) {
+    return VisitTemplateArgument(TA, std::forward<ParamTys>(P)...);
+  }
+  RetTy VisitNullPtrTemplateArgument(REF(TemplateArgument) TA, ParamTys... P) {
+    return VisitTemplateArgument(TA, std::forward<ParamTys>(P)...);
+  }
+  RetTy VisitIntegralTemplateArgument(REF(TemplateArgument) TA, ParamTys... P) {
+    return VisitTemplateArgument(TA, std::forward<ParamTys>(P)...);
+  }
+  RetTy VisitTemplateTemplateArgument(REF(TemplateArgument) TA, ParamTys... P) {
+    return VisitTemplateArgument(TA, std::forward<ParamTys>(P)...);
+  }
+  RetTy VisitTemplateExpansionTemplateArgument(REF(TemplateArgument) TA,
+                                               ParamTys... P) {
+    return VisitTemplateArgument(TA, std::forward<ParamTys>(P)...);
+  }
+  RetTy VisitExpressionTemplateArgument(REF(TemplateArgument) TA,
+                                        ParamTys... P) {
+    return VisitTemplateArgument(TA, std::forward<ParamTys>(P)...);
+  }
+  RetTy VisitPackTemplateArgument(REF(TemplateArgument) TA, ParamTys... P) {
+    return VisitTemplateArgument(TA, std::forward<ParamTys>(P)...);
+  }
+
+  RetTy VisitTemplateArgument(REF(TemplateArgument), ParamTys...) {
+    return RetTy();
+  }
+
+#undef REF
+#undef DISPATCH
+};
+
+} // namespace templateargumentvisitor
+
+/// A simple visitor class that helps create template argument visitors.
+///
+/// This class does not preserve constness of TemplateArgument references (see
+/// also ConstTemplateArgumentVisitor).
+template <typename ImplClass, typename RetTy = void, typename... ParamTys>
+class TemplateArgumentVisitor
+    : public templateargumentvisitor::Base<templateargumentvisitor::make_ref,
+                                           ImplClass, RetTy, ParamTys...> {};
+
+/// A simple visitor class that helps create template argument visitors.
+///
+/// This class preserves constness of TemplateArgument references (see also
+/// TemplateArgumentVisitor).
+template <typename ImplClass, typename RetTy = void, typename... ParamTys>
+class ConstTemplateArgumentVisitor
+    : public templateargumentvisitor::Base<
+          templateargumentvisitor::make_const_ref, ImplClass, RetTy,
+          ParamTys...> {};
+
+} // namespace clang
+
+#endif // LLVM_CLANG_AST_TEMPLATEARGUMENTVISITOR_H

include/clang/AST/TextNodeDumper.h

 #define LLVM_CLANG_AST_TEXTNODEDUMPER_H
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDumperUtils.h"
 #include "clang/AST/CommentCommandTraits.h"
 #include "clang/AST/CommentVisitor.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/StmtVisitor.h"
+#include "clang/AST/TemplateArgumentVisitor.h"
 #include "clang/AST/TypeVisitor.h"
 
 namespace clang {
 
 class TextNodeDumper
     : public comments::ConstCommentVisitor<TextNodeDumper, void,
                                            const comments::FullComment *>,
       public ConstStmtVisitor<TextNodeDumper>,
-      public TypeVisitor<TextNodeDumper> {
+      public TypeVisitor<TextNodeDumper>,
+      public ConstTemplateArgumentVisitor<TextNodeDumper> {
+
   TextTreeStructure &TreeStructure;
   raw_ostream &OS;
   const bool ShowColors;
 
   /// Keep track of the last location we print out so that we can
   /// print out deltas from then on out.
   const char *LastLocFilename = "";
   unsigned LastLocLine = ~0U;
void VisitGenericSelectionExprAssoc(const GenericSelectionExpr *E,
                                       unsigned I);
 
   void Visit(const Stmt *Node);
 
   void Visit(const Type *T);
 
   void Visit(QualType T);
 
+  void Visit(const TemplateArgument &TA, SourceRange R,
+             const Decl *From = nullptr, const char *label = nullptr);
+
   void dumpPointer(const void *Ptr);
   void dumpLocation(SourceLocation Loc);
   void dumpSourceRange(SourceRange R);
   void dumpBareType(QualType T, bool Desugar = true);
   void dumpType(QualType T);
   void dumpBareDeclRef(const Decl *D);
   void dumpName(const NamedDecl *ND);
   void dumpAccessSpecifier(AccessSpecifier AS);
public:
   void VisitTagType(const TagType *T);
   void VisitTemplateTypeParmType(const TemplateTypeParmType *T);
   void VisitAutoType(const AutoType *T);
   void VisitTemplateSpecializationType(const TemplateSpecializationType *T);
   void VisitInjectedClassNameType(const InjectedClassNameType *T);
   void VisitObjCInterfaceType(const ObjCInterfaceType *T);
   void VisitPackExpansionType(const PackExpansionType *T);
 
+  void VisitNullTemplateArgument(const TemplateArgument &TA);
+  void VisitTypeTemplateArgument(const TemplateArgument &TA);
+  void VisitDeclarationTemplateArgument(const TemplateArgument &TA);
+  void VisitNullPtrTemplateArgument(const TemplateArgument &TA);
+  void VisitIntegralTemplateArgument(const TemplateArgument &TA);
+  void VisitTemplateTemplateArgument(const TemplateArgument &TA);
+  void VisitTemplateExpansionTemplateArgument(const TemplateArgument &TA);
+  void VisitExpressionTemplateArgument(const TemplateArgument &TA);
+  void VisitPackTemplateArgument(const TemplateArgument &TA);
+
 private:
   void dumpCXXTemporary(const CXXTemporary *Temporary);
 };
 
 } // namespace clang
 
 #endif // LLVM_CLANG_AST_TEXTNODEDUMPER_H

lib/AST/ASTDumper.cpp

 #include "clang/AST/CommentVisitor.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclLookups.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclOpenMP.h"
 #include "clang/AST/DeclVisitor.h"
 #include "clang/AST/LocInfoType.h"
 #include "clang/AST/StmtVisitor.h"
+#include "clang/AST/TemplateArgumentVisitor.h"
 #include "clang/AST/TextNodeDumper.h"
 #include "clang/AST/TypeVisitor.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace clang;
 using namespace clang::comments;
 
 //===----------------------------------------------------------------------===//
 // ASTDumper Visitor
 //===----------------------------------------------------------------------===//
 
 namespace  {
 
   class ASTDumper
       : public ConstDeclVisitor<ASTDumper>,
         public ConstStmtVisitor<ASTDumper>,
         public ConstCommentVisitor<ASTDumper, void, const FullComment *>,
-        public TypeVisitor<ASTDumper> {
+        public TypeVisitor<ASTDumper>,
+        public ConstTemplateArgumentVisitor<ASTDumper> {
 
     TextTreeStructure TreeStructure;
     TextNodeDumper NodeDumper;
 
     raw_ostream &OS;
 
     /// The policy to use for printing; can be defaulted.
     PrintingPolicy PrintPolicy;
public:
     }
     void VisitSizeOfPackExpr(const SizeOfPackExpr *Node);
 
     // ObjC
     void VisitObjCAtCatchStmt(const ObjCAtCatchStmt *Node);
 
     // Comments.
     void dumpComment(const Comment *C, const FullComment *FC);
+
+    void VisitExpressionTemplateArgument(const TemplateArgument &TA) {
+      dumpStmt(TA.getAsExpr());
+    }
+    void VisitPackTemplateArgument(const TemplateArgument &TA) {
+      for (TemplateArgument::pack_iterator I = TA.pack_begin(),
+                                           E = TA.pack_end();
+           I != E; ++I)
+        dumpTemplateArgument(*I);
+    }
   };
 }
 
 //===----------------------------------------------------------------------===//
 //  Utilities
 //===----------------------------------------------------------------------===//
 
 void ASTDumper::dumpTypeAsChild(QualType T) {

 void ASTDumper::dumpTemplateArgumentList(const TemplateArgumentList &TAL) {
   for (unsigned i = 0, e = TAL.size(); i < e; ++i)
     dumpTemplateArgument(TAL[i]);
 }
 
 void ASTDumper::dumpTemplateArgument(const TemplateArgument &A, SourceRange R,
                                      const Decl *From, const char *label) {
   dumpChild([=] {
-    OS << "TemplateArgument";
-    if (R.isValid())
-      NodeDumper.dumpSourceRange(R);
-
-    if (From) {
-      NodeDumper.dumpDeclRef(From, label);
-    }
-
-    switch (A.getKind()) {
-    case TemplateArgument::Null:
-      OS << " null";
-      break;
-    case TemplateArgument::Type:
-      OS << " type";
-      NodeDumper.dumpType(A.getAsType());
-      break;
-    case TemplateArgument::Declaration:
-      OS << " decl";
-      NodeDumper.dumpDeclRef(A.getAsDecl());
-      break;
-    case TemplateArgument::NullPtr:
-      OS << " nullptr";
-      break;
-    case TemplateArgument::Integral:
-      OS << " integral " << A.getAsIntegral();
-      break;
-    case TemplateArgument::Template:
-      OS << " template ";
-      A.getAsTemplate().dump(OS);
-      break;
-    case TemplateArgument::TemplateExpansion:
-      OS << " template expansion ";
-      A.getAsTemplateOrTemplatePattern().dump(OS);
-      break;
-    case TemplateArgument::Expression:
-      OS << " expr";
-      dumpStmt(A.getAsExpr());
-      break;
-    case TemplateArgument::Pack:
-      OS << " pack";
-      for (TemplateArgument::pack_iterator I = A.pack_begin(), E = A.pack_end();
-           I != E; ++I)
-        dumpTemplateArgument(*I);
-      break;
-    }
+    NodeDumper.Visit(A, R, From, label);
+    ConstTemplateArgumentVisitor<ASTDumper>::Visit(A);
   });
 }
 
 //===----------------------------------------------------------------------===//
 //  Objective-C Utilities
 //===----------------------------------------------------------------------===//
 void ASTDumper::dumpObjCTypeParamList(const ObjCTypeParamList *typeParams) {
   if (!typeParams)

lib/AST/TextNodeDumper.cpp

 void TextNodeDumper::Visit(QualType T) {
   OS << "QualType";
   dumpPointer(T.getAsOpaquePtr());
   OS << " ";
   dumpBareType(T, false);
   OS << " " << T.split().Quals.getAsString();
 }
 
+void TextNodeDumper::Visit(const TemplateArgument &TA, SourceRange R,
+                           const Decl *From, const char *label) {
+  OS << "TemplateArgument";
+  if (R.isValid())
+    dumpSourceRange(R);
+
+  if (From) {
+    dumpDeclRef(From, label);
+  }
+
+  ConstTemplateArgumentVisitor<TextNodeDumper>::Visit(TA);
+}
+
 void TextNodeDumper::dumpPointer(const void *Ptr) {
   ColorScope Color(OS, ShowColors, AddressColor);
   OS << ' ' << Ptr;
 }
 
 void TextNodeDumper::dumpLocation(SourceLocation Loc) {
   if (!SM)
     return;

 void TextNodeDumper::VisitObjCInterfaceType(const ObjCInterfaceType *T) {
   dumpDeclRef(T->getDecl());
 }
 
 void TextNodeDumper::VisitPackExpansionType(const PackExpansionType *T) {
   if (auto N = T->getNumExpansions())
     OS << " expansions " << *N;
 }
+
+void TextNodeDumper::VisitNullTemplateArgument(const TemplateArgument &TA) {
+  OS << " null";
+}
+
+void TextNodeDumper::VisitTypeTemplateArgument(const TemplateArgument &TA) {
+  OS << " type";
+  dumpType(TA.getAsType());
+}
+
+void TextNodeDumper::VisitDeclarationTemplateArgument(
+    const TemplateArgument &TA) {
+  OS << " decl";
+  dumpDeclRef(TA.getAsDecl());
+}
+
+void TextNodeDumper::VisitNullPtrTemplateArgument(const TemplateArgument &TA) {
+  OS << " nullptr";
+}
+
+void TextNodeDumper::VisitIntegralTemplateArgument(const TemplateArgument &TA) {
+  OS << " integral " << TA.getAsIntegral();
+}
+
+void TextNodeDumper::VisitTemplateTemplateArgument(const TemplateArgument &TA) {
+  OS << " template ";
+  TA.getAsTemplate().dump(OS);
+}
+
+void TextNodeDumper::VisitTemplateExpansionTemplateArgument(
+    const TemplateArgument &TA) {
+  OS << " template expansion ";
+  TA.getAsTemplateOrTemplatePattern().dump(OS);
+}
+
+void TextNodeDumper::VisitExpressionTemplateArgument(
+    const TemplateArgument &TA) {
+  OS << " expr";
+}
+
+void TextNodeDumper::VisitPackTemplateArgument(const TemplateArgument &TA) {
+  OS << " pack";
+}