dblaikie added inline comments.
================ Comment at: clang/include/clang/AST/TemplateUtils.h:19 +namespace clang { +namespace TemplateUtils { +/// Make a best-effort determination of whether the type T can be produced by ---------------- aprantl wrote: > It looks like all the other namespaces in Clang are all lowercase? Yeah, +1 to the naming. & maybe not introducing a new "utils" bucket - they can be a bit of a dumping ground. Could this operation belong elsewhere/in some existing type, or maybe doesn't need a namespace wrapper - could be a free function in the `clang` namespace?) ================ Comment at: clang/lib/AST/TemplateUtils.cpp:19 + +namespace { +bool isSubstitutedTemplateArgument(ASTContext &Ctx, TemplateArgument Arg, ---------------- The LLVM style is to use `static` for file-local functions, only using anonymous namespaces for file-local classes ( https://llvm.org/docs/CodingStandards.html#anonymous-namespaces ) ================ Comment at: clang/lib/AST/TemplateUtils.cpp:144-148 + } else if (auto *TTPD = dyn_cast<TemplateTemplateParmDecl>(Param)) { + return TTPD->hasDefaultArgument() && + isSubstitutedTemplateArgument( + Ctx, Arg, TTPD->getDefaultArgument().getArgument(), Args, Depth); + } else if (auto *NTTPD = dyn_cast<NonTypeTemplateParmDecl>(Param)) { ---------------- Could probably remove these "else after return"s while you're moving this code if you like. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139985/new/ https://reviews.llvm.org/D139985 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits