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

[Tobias Ribizel via Phabricator via cfe-commits]

upsj added a comment.

> ! In D124690#3493246 <https://reviews.llvm.org/D124690#3493246>, @sammccall wrote:
>
> This makes sense to me.
>  This sounds more complicated, but if I'm reading correctly this multi-level forwarding isn't handled in the current version of the patch either?
>  At some point we'll want to extract this logic out so it can be used by hover etc, but not yet.

Yes, currently only direct forwarding works. I originally looked at this when inlay hints were not yet available, and it seemed possible to do this for signature help as well, only most likely based on the template instantiation pattern instead of the instantiated function declaration.



================
Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:255
+  )cpp",
+                       ExpectedHint{"a: ", "param"});
+}
----------------
sammccall wrote:
> upsj wrote:
> > I haven't been able to figure out why, but for some reason the `CXXConstructExpr` and `CXXTemporaryObjectExpr` are not being picked up by the `RecursiveASTVisitor`, while in a similar situation below, the `CallExpr` gets picked up by `VisitCallExpr`.
> The AST here of the instantiated bar looks like
> ```
> -FunctionDecl <line:5:1, line:7:1> line:5:4 used bar 'S *(int &&)'
>   |-TemplateArgument type 'S'
>   |-TemplateArgument pack
>   | `-TemplateArgument type 'int'
>   |-ParmVarDecl <col:8, col:18> col:18 used args 'int &&'
>   `-CompoundStmt <col:24, line:7:1>
>     `-ReturnStmt <line:6:3, col:23>
>       `-CXXNewExpr <col:10, col:23> 'S *' Function 'operator new' 'void *(unsigned long)'
>         `-CXXConstructExpr <col:14, col:23> 'S':'S' 'void (int)' list
>           `-ImplicitCastExpr <col:16> 'int':'int' <LValueToRValue>
>             `-DeclRefExpr <col:16> 'int':'int' lvalue ParmVar 'args' 'int &&'
> ```
> 
> So there's no CXXTemporaryObjectExpr (the value here is a pointer), but should be a CXXConstructExpr. 
> 
> Are you sure you're traversing bar's instantiation/specializaion, as opposed to its templated/generic FunctionDecl? The AST of the templated FunctionDecl indeed has an InitListExpr in place of the CXXConstructExpr because it's not resolved yet.
It's quite possible this is related to the visitor not/only inconsistently traversing template instantiations due to shouldVisitTemplateInstantiations returning false. But I need to implement the new approach first, anyways.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124690/new/

https://reviews.llvm.org/D124690