nridge added a comment.
Had a read through this. I'm still digesting it, but the high-level approach
seems reasonable to me.
Could we add a test case for the recursive scenario that came up during chat:
void foo();
template <typename Head, typename... Tail>
void foo(Head head, Tail... tail) {
foo(tail...);
}
int main() {
foo(1, 2, 3);
}
`
(even if the behaviour on this testcase isn't what we want yet, it's useful to
have it in the test suite as a reminder)
================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:344
+
+ const DeclRefExpr *unpackArgument(const Expr *E) {
+ E = unpackImplicitCast(E);
----------------
unwrap? "unpack" sounds like something related to parameter packs
================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:374
+ // have one in the definition, store this mapping here.
+ llvm::DenseMap<const ParmVarDecl *, const ParmVarDecl *> ParamDeclToDef;
+};
----------------
I think it would be more readable if the state that persists across calls
(TraversedFunctions, ForwardedParams, ParamDeclToDef) would be separate from
the state that does not (UnresolvedParams, TraversalQueue).
A concrete suggestion:
* Factor the first set out into a `ParameterMappingCache` struct
* Do not keep a `ForwardingParameterVisitor` as a member of
`InlayHintVisitor`, only the `ParameterMappingCache`
* Create the `ForwardingParameterVisitor` as a local for each call, and pass
it a reference to the `ParameterMappingCache` in the constructor
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
