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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
