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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to