nridge added inline comments.
================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:208
+ // If the parameter is part of an expanded pack and not yet resolved
+ if (/*isExpandedParameter(Param) && */
+ ForwardedParams.find(Param) == ForwardedParams.end()) {
----------------
nridge wrote:
> upsj wrote:
> > This needs to be fixed, see `ParameterHints.VariadicPlain` vs.
> > `ParameterHints.VariadicForwarded` if uncommented - I'd need some input
> > from somebody with more knowledge about the AST
> It looks like `isExpandedParameter()` relies on the
> `SubstTemplateTypeParmType` type sugar being present in the ParmVarDecl's
> type, but in some cases, the ParmVarDecl's type points to the canonical type
> directly.
>
> I'm not sure what sort of guarantees the AST intends to make about the
> presence of type sugar. Based on past experience with Eclipse CDT, it's very
> easy to lose type sugar and maintaining it in all the right places takes some
> effort.
Upon further investigation, it looks like the ParmVarDecl is retaining the type
sugar fine, it's the `getNonReferenceType()` call in `isExpandedParameter()`
that loses it.
What happens with perfect forwarding when the argument is an lvalue is a bit
subtle. In this testcase:
```
template <typename... Args>
void bar(Args&&... args) { return foo(std::forward<Args>(args)...); }
void baz() {
int b;
bar($param[[b]]);
}
```
the template argument that `bar()` is instantiated with is `Args = [int &]`.
Substituting into `Args&&`, that then becomes `int& &&` which collapses into
`int&`, leaving the instantiated parameter type an lvalue reference type.
Clang does in fact model this accurately, which means the type structure is:
```
BuiltinType
ReferenceType
SubstTemplateTypeParmType
ReferenceType
```
The outer reference type is the `&&` that's outside the `Args`, the
`SubstTemplateTypeParmType` reflects the substitution `Args = int&`, and the
inner `ReferenceType` is the `int&`.
The problem is, `getNonReferenceType()` unpacks _all_ the reference types,
skipping past the `SubstTemplateTypeParmType` and giving you the `BuiltinType`.
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
