aaron.ballman added inline comments.
================ Comment at: include/clang/AST/TemplateArgumentVisitor.h:23 + +template <typename T> struct make_ref { using type = T &; }; +template <typename T> struct make_const_ref { using type = const T &; }; ---------------- `std::add_lvalue_reference<>` already does this, so do we really need this? ================ Comment at: include/clang/AST/TemplateArgumentVisitor.h:24 +template <typename T> struct make_ref { using type = T &; }; +template <typename T> struct make_const_ref { using type = const T &; }; + ---------------- 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). ================ Comment at: include/clang/AST/TemplateArgumentVisitor.h:27-28 +/// A simple visitor class that helps create template argument visitors. +template <template <typename> class Ref, typename ImplClass, + typename RetTy = void, class... ParamTys> +class Base { ---------------- s/class/typename to be self-consistent ================ Comment at: include/clang/AST/TemplateArgumentVisitor.h:55 + + RetTy VisitNullTemplateArgument(REF(TemplateArgument) TA, ParamTys... P) { + return VisitTemplateArgument(TA, std::forward<ParamTys>(P)...); ---------------- Might be worth it to replace these with a macro, sort of like how `DISPATCH` is used above. ================ Comment at: include/clang/AST/TextNodeDumper.h:156 + void Visit(const TemplateArgument &TA, SourceRange R, + const Decl *From = nullptr, const char *label = nullptr); + ---------------- label -> Label and I think we should use a `StringRef` for the type. ================ Comment at: lib/AST/ASTDumper.cpp:449-450 + void VisitPackTemplateArgument(const TemplateArgument &TA) { + for (TemplateArgument::pack_iterator I = TA.pack_begin(), + E = TA.pack_end(); + I != E; ++I) ---------------- `llvm::for_each(TA.pack_elements(), ...);` (or a range-based for loop) ================ Comment at: lib/AST/TextNodeDumper.cpp:68 +void TextNodeDumper::Visit(const TemplateArgument &TA, SourceRange R, + const Decl *From, const char *label) { + OS << "TemplateArgument"; ---------------- label -> Label ================ Comment at: lib/AST/TextNodeDumper.cpp:73 + + if (From) { + dumpDeclRef(From, label); ---------------- Elide braces Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55491/new/ https://reviews.llvm.org/D55491 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits