[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

This revision was automatically updated to reflect the committed changes. Closed by commit rGb252824e6e6e: [clangd] fix wrong CalleeArgInfo in the hover (authored by v1nh1shungry). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142014/new/

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

tom-anders added a comment. In D142014#4106088 , @v1nh1shungry wrote: > Thanks for the review! @kadircet > > Would you mind having a look at if there're any concerns about the current > code change, @nridge, @tom-anders, and @adamcz? Thanks a lot!

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry added a comment. Thanks for the review! @kadircet Would you mind having a look at if there're any concerns about the current code change, @nridge, @tom-anders, and @adamcz? Thanks a lot! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142014/new/ https://reviews.llvm.org/D142014

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry updated this revision to Diff 493626. v1nh1shungry added a comment. - restore code changes - add test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142014/new/ https://reviews.llvm.org/D142014 Files:

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:994 HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, PP)); + PassType.PassBy = getPassMode(PVD->getType()); +} kadircet wrote: > v1nh1shungry wrote: > > kadircet

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:994 HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, PP)); + PassType.PassBy = getPassMode(PVD->getType()); +} v1nh1shungry wrote: > kadircet wrote: > > v1nh1shungry

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:994 HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, PP)); + PassType.PassBy = getPassMode(PVD->getType()); +} kadircet wrote: > v1nh1shungry wrote: > > kadircet

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:994 HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, PP)); + PassType.PassBy = getPassMode(PVD->getType()); +} v1nh1shungry wrote: > kadircet wrote: > > v1nh1shungry

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry added a comment. In D142014#4084263 , @nridge wrote: > In D142014#4081922 , @v1nh1shungry > wrote: > >> I just came up with a case where the original implementation and this patch >> behave

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

nridge added a comment. In D142014#4081922 , @v1nh1shungry wrote: > I just came up with a case where the original implementation and this patch > behave differently. > > void foobar(const float &); > int main() { > foobar(0); >^ >

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:994 HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, PP)); + PassType.PassBy = getPassMode(PVD->getType()); +} v1nh1shungry wrote: > kadircet wrote: > > sorry for

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry added a comment. I just came up with a case where the original implementation and this patch behave differently. void foobar(const float &); int main() { foobar(0); ^ } Used to `Passed by value`, now it is `Passed by const reference`. I think the former one

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:994 HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, PP)); + PassType.PassBy = getPassMode(PVD->getType()); +} kadircet wrote: > sorry for showing up late to the

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:994 HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, PP)); + PassType.PassBy = getPassMode(PVD->getType()); +} sorry for showing up late to the party but instead of

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry marked an inline comment as done. v1nh1shungry added a comment. Thanks for the review! @tom-anders Comment at: clang-tools-extra/clangd/Hover.cpp:1035 } else if (const auto *MTE = CastNode->ASTNode.get()) { } else { // Unknown

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry updated this revision to Diff 491594. v1nh1shungry added a comment. land review's suggestions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142014/new/ https://reviews.llvm.org/D142014 Files: clang-tools-extra/clangd/Hover.cpp

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry marked an inline comment as done. v1nh1shungry added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1035 } else if (const auto *MTE = CastNode->ASTNode.get()) { } else { // Unknown implicit node, assume type conversion.

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1035 } else if (const auto *MTE = CastNode->ASTNode.get()) { } else { // Unknown implicit node, assume type conversion. v1nh1shungry wrote: >

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1035 } else if (const auto *MTE = CastNode->ASTNode.get()) { } else { // Unknown implicit node, assume type conversion. tom-anders wrote: > This

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

tom-anders added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1035 } else if (const auto *MTE = CastNode->ASTNode.get()) { } else { // Unknown implicit node, assume type conversion. This branch is now empty, do we

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry marked an inline comment as done. v1nh1shungry added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142014/new/ https://reviews.llvm.org/D142014 ___ cfe-commits mailing

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry updated this revision to Diff 491440. v1nh1shungry added a comment. `==` to `>=` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142014/new/ https://reviews.llvm.org/D142014 Files: clang-tools-extra/clangd/Hover.cpp

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

nridge added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1030 + } else { +if (CD->getNumParams() == 1) + PassType.PassBy = getPassMode(CD->getParamDecl(0)->getType()); This check should be `>= 1`, since the constructor

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry marked 2 inline comments as done. v1nh1shungry added a comment. Thanks for the review! @nridge Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142014/new/ https://reviews.llvm.org/D142014 ___

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry updated this revision to Diff 491287. v1nh1shungry added a comment. land comment's suggestions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142014/new/ https://reviews.llvm.org/D142014 Files: clang-tools-extra/clangd/Hover.cpp

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

nridge added a subscriber: adamcz. nridge added a comment. Thanks for the patch! I had a look at the code history, and it looks like the original reason for not using the param decl's type to determine the passing mode was (based on this comment )

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

v1nh1shungry created this revision. v1nh1shungry added reviewers: tom-anders, nridge. Herald added subscribers: kadircet, arphaman. Herald added a project: All. v1nh1shungry requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: