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

23 ↗

(On Diff #180947)

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

24 ↗

(On Diff #180947)

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).

27–28 ↗

(On Diff #180947)

s/class/typename to be self-consistent

55 ↗

(On Diff #180947)

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

include/clang/AST/TextNodeDumper.h

156 ↗

(On Diff #180947)

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

lib/AST/ASTDumper.cpp

449–450 ↗

(On Diff #180947)

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

lib/AST/TextNodeDumper.cpp

68 ↗

(On Diff #180947)

label -> Label

73 ↗

(On Diff #180947)

Elide braces

[steveire]

lib/AST/ASTDumper.cpp

449–450 ↗

(On Diff #180947)

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

449–450 ↗

(On Diff #180947)

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

24 ↗

(On Diff #181395)

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

24 ↗

(On Diff #181395)

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

cfe/trunk/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 {
+
+/// A simple visitor class that helps create template argument visitors.
+template <template <typename> class Ref, typename ImplClass,
+          typename RetTy = void, typename... 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.
+
+#define VISIT_METHOD(CATEGORY)                                                 \
+  RetTy Visit##CATEGORY##TemplateArgument(REF(TemplateArgument) TA,            \
+                                          ParamTys... P) {                     \
+    return VisitTemplateArgument(TA, std::forward<ParamTys>(P)...);            \
+  }
+
+  VISIT_METHOD(Null);
+  VISIT_METHOD(Type);
+  VISIT_METHOD(Declaration);
+  VISIT_METHOD(NullPtr);
+  VISIT_METHOD(Integral);
+  VISIT_METHOD(Template);
+  VISIT_METHOD(TemplateExpansion);
+  VISIT_METHOD(Expression);
+  VISIT_METHOD(Pack);
+
+  RetTy VisitTemplateArgument(REF(TemplateArgument), ParamTys...) {
+    return RetTy();
+  }
+
+#undef REF
+#undef DISPATCH
+#undef VISIT_METHOD
+};
+
+} // 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<std::add_lvalue_reference, 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<llvm::make_const_ref, ImplClass,
+                                           RetTy, ParamTys...> {};
+
+} // namespace clang
+
+#endif // LLVM_CLANG_AST_TEMPLATEARGUMENTVISITOR_H

cfe/trunk/include/clang/AST/TextNodeDumper.h

 #define LLVM_CLANG_AST_TEXTNODEDUMPER_H
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDumperUtils.h"
 #include "clang/AST/AttrVisitor.h"
 #include "clang/AST/CommentCommandTraits.h"
 #include "clang/AST/CommentVisitor.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/TemplateArgumentVisitor.h"
 
 namespace clang {
 
 class TextTreeStructure {
   raw_ostream &OS;
   const bool ShowColors;
 
   /// Pending[i] is an action to dump an entity at level i.
public:
   TextTreeStructure(raw_ostream &OS, bool ShowColors)
       : OS(OS), ShowColors(ShowColors) {}
 };
 
 class TextNodeDumper
     : public TextTreeStructure,
       public comments::ConstCommentVisitor<TextNodeDumper, void,
                                            const comments::FullComment *>,
-      public ConstAttrVisitor<TextNodeDumper> {
+      public ConstAttrVisitor<TextNodeDumper>,
+      public ConstTemplateArgumentVisitor<TextNodeDumper> {
   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;
 
public:
   TextNodeDumper(raw_ostream &OS, bool ShowColors, const SourceManager *SM,
                  const PrintingPolicy &PrintPolicy,
                  const comments::CommandTraits *Traits);
 
   void Visit(const comments::Comment *C, const comments::FullComment *FC);
 
   void Visit(const Attr *A);
 
+  void Visit(const TemplateArgument &TA, SourceRange R,
+             const Decl *From = nullptr, StringRef Label = {});
+
   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
   visitVerbatimBlockLineComment(const comments::VerbatimBlockLineComment *C,
                                 const comments::FullComment *);
   void visitVerbatimLineComment(const comments::VerbatimLineComment *C,
                                 const comments::FullComment *);
 
 // Implements Visit methods for Attrs.
 #include "clang/AST/AttrTextNodeDump.inc"
+
+  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);
 };
 
 } // namespace clang
 
 #endif // LLVM_CLANG_AST_TEXTNODEDUMPER_H

cfe/trunk/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 ConstAttrVisitor<ASTDumper> {
+        public ConstAttrVisitor<ASTDumper>,
+        public ConstTemplateArgumentVisitor<ASTDumper> {
 
     TextNodeDumper NodeDumper;
 
     raw_ostream &OS;
 
     /// The policy to use for printing; can be defaulted.
     PrintingPolicy PrintPolicy;
 
public:
     void VisitObjCPropertyRefExpr(const ObjCPropertyRefExpr *Node);
     void VisitObjCSubscriptRefExpr(const ObjCSubscriptRefExpr *Node);
     void VisitObjCIvarRefExpr(const ObjCIvarRefExpr *Node);
     void VisitObjCBoolLiteralExpr(const ObjCBoolLiteralExpr *Node);
 
     // Comments.
     void dumpComment(const Comment *C, const FullComment *FC);
 
+    void VisitExpressionTemplateArgument(const TemplateArgument &TA) {
+      dumpStmt(TA.getAsExpr());
+    }
+    void VisitPackTemplateArgument(const TemplateArgument &TA) {
+      for (const auto& TArg : TA.pack_elements())
+        dumpTemplateArgument(TArg);
+    }
+
 // Implements Visit methods for Attrs.
 #include "clang/AST/AttrNodeTraverse.inc"
   };
 }
 
 //===----------------------------------------------------------------------===//
 //  Utilities
 //===----------------------------------------------------------------------===//

 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 (const auto& TArg : A.pack_elements())
-        dumpTemplateArgument(TArg);
-      break;
-    }
+    NodeDumper.Visit(A, R, From, Label);
+    ConstTemplateArgumentVisitor<ASTDumper>::Visit(A);
   });
 }
 
 //===----------------------------------------------------------------------===//
 //  Objective-C Utilities
 //===----------------------------------------------------------------------===//
 void ASTDumper::dumpObjCTypeParamList(const ObjCTypeParamList *typeParams) {
   if (!typeParams)

cfe/trunk/lib/AST/TextNodeDumper.cpp

#include "clang/Basic/AttrList.inc"
   if (A->isInherited())
     OS << " Inherited";
   if (A->isImplicit())
     OS << " Implicit";
 
   ConstAttrVisitor<TextNodeDumper>::Visit(A);
 }
 
+void TextNodeDumper::Visit(const TemplateArgument &TA, SourceRange R,
+                           const Decl *From, StringRef 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::visitVerbatimBlockLineComment(
     const comments::FullComment *) {
   OS << " Text=\"" << C->getText() << "\"";
 }
 
 void TextNodeDumper::visitVerbatimLineComment(
     const comments::VerbatimLineComment *C, const comments::FullComment *) {
   OS << " Text=\"" << C->getText() << "\"";
 }
+
+void TextNodeDumper::VisitNullTemplateArgument(const TemplateArgument &) {
+  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 &) {
+  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 &) {
+  OS << " expr";
+}
+
+void TextNodeDumper::VisitPackTemplateArgument(const TemplateArgument &) {
+  OS << " pack";
+}