mizvekov added inline comments.
================ Comment at: clang/include/clang/AST/DeclTemplate.h:3427 +TemplateParameterList *getReplacedTemplateParameterList(Decl *D); + ---------------- davrec wrote: > davrec wrote: > > I don't object with making this public, and I can see the argument for > > making this its own function rather than a Decl method all things being > > equal, but given that we already have `Decl::getDescribedTemplateParams`, I > > think that > > # this should probably also go in Decl, > > # a good comment is needed explaining the difference between the two (e.g. > > that the `D` is the template/template-like decl itself, not a templated > > decl as required by `getDescribedTemplateParams`, or whatever), and what > > happens when it is called on a non-template decl, and > > # it probably should be named just `getTemplateParams` or something that > > suggests its difference with `getDescribedTemplateParams`. > > > On second thought this should definitely be a Decl method called > `getTemplateParameters()`, since all it does is dispatches to the derived > class's implementation of that. I think this function shouldn't be public in that sense, it should only be used for implementation of retrieving parameter for Subst* nodes. I don't think this should try to handle any other kinds of decls which can't appear as the AssociatedDecl in a Subst node. There will be Subst specific changes here in the future, but which depend on some other enablers: * We need to make Subst nodes for variable templates store the SpecializationDecl instead of the TemplateDecl, but this requires changing the order between creating the specialization and performing the substitution. I will do that in a separate patch. * We will stop creating Subst* nodes for things we already performed substitution with sugar. And so, we won't need to handle alias templates, conceptdecls, etc. Subst nodes will only be used for things which we track specializations for, and that need resugaring. It's only made 'public' because now we are using it across far away places like `Type.cpp` and `ExprCXX.cpp` and `TemplateName.cpp`. I didn't think this needs to go as a Decl member, because of limited scope and because it only ever needs to access public members. I don't think this justifies either a separate header to be included only where it's needed. One option is to further make the name more specific, by adding `Internal` to the name for example. Any other ideas? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131858/new/ https://reviews.llvm.org/D131858 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
