[PATCH] D139985: [clang][AST][NFC] Move isSubstitutedDefaultArgument out of TypePrinter into separate component
David Blaikie via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 14 10:13:07 PST 2022
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
More information about the cfe-commits
mailing list