[PATCH] D124690: [clangd] add inlay hints for std::forward-ed parameter packs

[Nathan Ridge via Phabricator via cfe-commits]

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