erichkeane accepted this revision.
erichkeane added a comment.
A couple of nits, otherwise I don't have to see this again.
================
Comment at: clang/include/clang/AST/TemplateName.h:158
+ // When true the substitution will be finalized (subst node won't be placed).
+ bool getFinal() const;
+
----------------
Based on this comment, should this be 'Finalize' instead of 'Final'? Or
something like "ShouldFinalize"?
================
Comment at: clang/include/clang/Sema/Template.h:104
/// Construct a single-level template argument list.
- MultiLevelTemplateArgumentList(Decl *D, ArgList Args) {
- addOuterTemplateArguments(D, Args);
+ MultiLevelTemplateArgumentList(Decl *D, ArgList Args, bool Final) {
+ addOuterTemplateArguments(D, Args, Final);
----------------
Hrmph, this additional flag likely requires rebasing/rebuilding on a patch I
just committed, which uses this MLTAL again. Hopefully not too bad to
propagate.
================
Comment at: clang/lib/AST/ASTContext.cpp:3045
+static bool
+getCanonicalTemplateArguments(const ASTContext &C,
+ ArrayRef<TemplateArgument> OrigArgs,
----------------
typically we name these sorts of things based on their return type, so this
would be more `hasCanonicalTemplateArguments`. BUT the modifying nature of it
makes this an awkward function all the same.
I think I'd suggest putting both of the 'output's here on the same side of the
function, either both a return, or both an out-param, so the name isn't
ambiguous/confusing like this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134604/new/
https://reviews.llvm.org/D134604
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
