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

sammccall added a comment. In D130260#3671494 , @kadircet wrote: > In D130260#3671290 , @sammccall > wrote: > >> driveby thoughts, but please don't block on them. >> >> (if this fix is a heuristic that fixes a

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG4839929bed6e: [clangd] Make forwarding parameter detection logic resilient (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D130260?vs=446796=446805#toc Repository: rG LLVM

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

ilya-biryukov accepted this revision. ilya-biryukov added a comment. LGTM Comment at: clang-tools-extra/clangd/AST.cpp:830 +assert(Parameters.size() <= + static_cast(std::distance(Args.begin(), Args.end(; +for (auto Begin = Args.begin(), End = Args.end()

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

kadircet added a comment. In 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

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

kadircet updated this revision to Diff 446796. kadircet marked 5 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130260/new/ https://reviews.llvm.org/D130260 Files:

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

sammccall added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:832 continue; +// Check that this expands all the way until the last parameter. +auto ParamEnd = It + Parameters.size() - 1; upsj wrote: > sammccall wrote: >

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

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:828 // find the argument directly referring to the first parameter -for (auto It = Args.begin(); It != Args.end(); ++It) { - const Expr *Arg = *It; - if (const auto *RefArg =

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

upsj added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:832 continue; +// Check that this expands all the way until the last parameter. +auto ParamEnd = It + Parameters.size() - 1; sammccall wrote: > The essence of

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

sammccall added a comment. 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) Comment at: clang-tools-extra/clangd/AST.cpp:832

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

kadircet updated this revision to Diff 446770. kadircet added a comment. - Update test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130260/new/ https://reviews.llvm.org/D130260 Files: clang-tools-extra/clangd/AST.cpp

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

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:833 +// Check that this expands all the way until the last parameter. +auto ParamEnd = It + Parameters.size() - 1; +assert(std::distance(Args.begin(), ParamEnd) <

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

kadircet updated this revision to Diff 446769. kadircet marked an inline comment as done. kadircet added a comment. - Rather than asserting limit the traversal - Have more comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130260/new/

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

upsj accepted this revision. upsj added a comment. This revision is now accepted and ready to land. Not sure I qualify as a suitable reviewer, but this looks really good to me, save for maybe a small safety measure :) Comment at: clang-tools-extra/clangd/AST.cpp:833-837 +

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

kadircet added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:790 +// Skip functions with less parameters, they can't be the target. +if (Callee->parameters().size() < Parameters.size()) + return; upsj wrote: > This is not a

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

kadircet updated this revision to Diff 446763. kadircet added a comment. - Add OOB check as an asssertion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130260/new/ https://reviews.llvm.org/D130260 Files: clang-tools-extra/clangd/AST.cpp

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

kadircet updated this revision to Diff 446759. kadircet added a comment. - Check to ensure function call receives all the arguments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130260/new/ https://reviews.llvm.org/D130260 Files:

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

upsj added a comment. Your approach to handling implicit conversions is much nicer than mine, so I abandoned my revision. There is still one case (that might even occur in the code base I'm working on?) that this change would lead to incorrect hints on. WDYT about keeping the other changes,

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

kadircet updated this revision to Diff 446531. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130260/new/ https://reviews.llvm.org/D130260 Files: clang-tools-extra/clangd/AST.cpp

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

kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang-tools-extra. This could