adamcz added a comment. Added a slightly awkward clang (not clangd) test. The output is suboptimial, because it's not clangd, but it shows things work as they should.
================ Comment at: clang-tools-extra/clangd/CodeComplete.cpp:929 + // + // This only matters for variadic functions/templates, where CurrentArg + // might be higher than the number of parameters. When that happens, we ---------------- sammccall wrote: > hmm, now that I think about it, should this conversion from arg-index to > param-index be happening here, or done already by sema and the param index > passed to ProcessOverloadCandidates? > > (The docs say "current arg" but I'm struggling to see how that can really be > useful) Since ProcessOverloadCandidates is called with multiple candidates CurrentParam wouldn't make sense - which candidate would that be referring to? What if the candidates are: void fun1(int x, ...); void fun2(int x, int y, ...); and CurrentArg is 3, that means for the first signature activeParameter = 1, while for the second one activeParameter = 2. Right now we do not return this information, but we should (I left a FIXME and intend to follow up, LSP already has the per-signature field). We could consider moving the CurrentArg out of this function signature and into the OverloadCandidate, so it's per candidate in Sema too. Is that what you're suggesting? I think the current state of things is fine, but if you think I should remove CurrentArg and replace it with a field in OverloadCandidate, let me know. ================ Comment at: clang-tools-extra/clangd/CodeComplete.cpp:945 + } + SigHelp.activeParameter = + std::min(SigHelp.activeParameter, NumParams - 1); ---------------- sammccall wrote: > hmm, if foo() is a non-variadic 0-arg function, we set activeParameter to -1. > Before this patch it's zero which is still weird, but less weird. Right, that's unintended. Fixed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111318/new/ https://reviews.llvm.org/D111318 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
