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

Vincent Hong via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 23 20:25:15 PST 2023


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<MaterializeTemporaryExpr>()) {
     } else { // Unknown implicit node, assume type conversion.
----------------
v1nh1shungry wrote:
> tom-anders wrote:
> > v1nh1shungry wrote:
> > > tom-anders wrote:
> > > > This branch is now empty, do we still need it? 
> > > I keep this branch because the original implementation doesn't want to mark `Converted` in this branch. Yeah I can modify the following `else` to `else if (const ...; !MTE)` (or something cleaner), but I think the two are the same ugly :(
> > Ah I see, what about just adding `PassType.Converted = false` here (even though it has no effect, but just to make this more clear)? 
> Then how about just adding some comments? I don't have a strong opinion here though.
Done. Sorry for the confusion here! Hope it is clear now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142014



More information about the cfe-commits mailing list