nridge added a comment.
Thanks for the update and the additional test cases!
================
Comment at: clang-tools-extra/clangd/AST.cpp:682
+ if (const auto *TTPD =
+ dyn_cast<TemplateTypeParmDecl>(TemplateParams.back())) {
+ const auto *TTPT =
----------------
upsj wrote:
> nridge wrote:
> > I don't think there is any requirement that a pack be a trailing
> > **template** parameter. For example, the following is valid:
> >
> > ```
> > template <typename... B, typename A>
> > void foo(A, B...);
> >
> > void bar() {
> > foo(1, 2, 3);
> > }
> > ```
> Do you have a suggestion for how to find this pack in general? I would like
> to keep this function as efficient as possible, since it's used everywhere
> Do you have a suggestion for how to find this pack in general?
Just iterate backwards through `TemplateParams` rather than only considering
`TemplateParams.back()`, I suppose.
> I would like to keep this function as efficient as possible, since it's used
> everywhere
The `ParmVarDecl*` overload of `getPackTemplateParameter()` is called a lot via
the `IsExpandedPack` lambdas, but this overload is only called once per depth
level.
================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:541
+ // Remove parameter names that occur multiple times completely.
+ llvm::StringMap<size_t> NameLastSeen;
----------------
upsj wrote:
> nridge wrote:
> > This is an interesting approach for handling `VariadicRecursive`.
> >
> > I had in mind a different approach, something like keeping a
> > `std::set<FunctionTemplateDecl*> SeenFunctionTemplates` in
> > `resolveForwardingParameters()`, populating it with
> > `CurrentFunction->getPrimaryTemplate()` on each iteration, and bailing if
> > the same function template is seen more than once (indicating recursion).
> > But this approach seems to work too, as a parameter name can't legitimately
> > appear twice in a function declaration.
> >
> > That said, maybe having such a `SeenFunctionTemplates` recursion guard
> > would be helpful anyways, so that e.g. in `VariadicInfinite`, we would bail
> > after a single recursion rather than going until `MaxDepth`?
> I see your point here - I would also like an AST based approach more than
> this purely string-based one. The main issue is that if I deduplicate based
> on the function templates, I would still be left with the first parameter
> being named, which doesn't make much sense in something like make_tuple.
One idea is that we could return the original `Parameters` from
`resolveFowardingParameters()` if we encounter recursion.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124690/new/
https://reviews.llvm.org/D124690
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
