upsj added inline comments.
================
Comment at: clang-tools-extra/clangd/AST.cpp:832
continue;
+ // Check that this expands all the way until the last parameter.
+ auto ParamEnd = It + Parameters.size() - 1;
----------------
sammccall wrote:
> The essence of this test seems to be "if we can't rule it out by counting
> parameters, it must be right" which... doesn't seem very robust? Am I missing
> something?
>
> AIUI the idea here is that
> A) `foo(pack...)` is "expanded all the way" into consecutive arguments
> B) `foo(pack)...` is not considered forwarding for our purposes
> C) we want to treat `foo(bar(pack)...)` specially if `bar` is "just a cast"
> like move or forward
>
> The difference between cases A and B aren't about counting arguments though,
> they're about what operations occur intercede between `pack` and the `...`
> operator. (i.e. `foo((&pack)...)` is fine for the same reason `forward` is.
>
> This seems like something best handled by:
> - finding the `PackExpansionExpr` controlling `pack` in the non-instantiated
> template AST
> - walking down to try to find `pack`
> - continue as we walk through e.g. parens or std::forward
> - bail out if we walk through something unknown (e.g. another function call)
>
> Does this seem sensible/feasible?
> (Walking up from `pack` is nicer but the AST doesn't support walking up
> well...)
This goes a bit more generally into the question how to approach parameter
packs across clangd. For inlay hints and hover, we are already looking at
instantiated templates (is that true in all cases? Haven't dug in yet), so
implementing forwarding there was straightforward. For signature help and code
completion, we'd probably need to go via the uninstantiated template (and
probably lose a bit of type information on the way, e.g. reference vs. value,
l-value vs. r-value). If we wanted a generic way to deal with forwarding, the
instantiation pattern would be my first choice, but I didn't have the time to
go down this rabbit hole yet.
Unless there is a nice way to map between template and instantiation AST, I
don't think combining them is feasible.
On the heuristic itself: Is there a way in C++ to "partially" unpack a
parameter pack without this showing up in the AST? I am not aware of any, which
is why the test "do the first and last parameters appear in a matching range of
the arguments" seems sufficient and necessary in all but single-entry parameter
packs.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130260/new/
https://reviews.llvm.org/D130260
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
