[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

Fri Jul 22 05:45:26 PDT 2022

sammccall added a comment.

In D130260#3671494 <https://reviews.llvm.org/D130260#3671494>, @kadircet wrote:

> In D130260#3671290 <https://reviews.llvm.org/D130260#3671290>, @sammccall wrote:
>
>> driveby thoughts, but please don't block on them.
>>
>> (if this fix is a heuristic that fixes a common crash but isn't completely correct, that may still be worth landing but warrants a fixme)
>
> The fix is not an heuristic. We just make sure we never consider a function with less parameters than the arguments we have, that way rest of the assumptions hold (we might still pick a wrong function call, as we could in the past, but we won't crash).
> In addition to that heuristics are improved a little bit to work in presence of copy/move constructors.

OK, but it's missing a comment that says "this is a heuristic and may not be correct, at least it won't crash"!

The original code was intended to be *correct*, not a heuristic. If we're not going to fix it properly (which is reasonable in the circumstances), we need to document the limitations.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130260