[PATCH] D92041: [clangd] Add hover info for `this` expr

xndcn added a comment. In D92041#2454236 , @sammccall wrote: > Very nice, thanks! > I'll land this for you now. Thank you very much! Learned a lot about clangd and clang AST from this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D92041: [clangd] Add hover info for `this` expr

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG9c328e7afafd: [clangd] Add hover info for `this` expr (authored by xndcn, committed by sammccall). Repository: rG LLVM

[PATCH] D92041: [clangd] Add hover info for `this` expr

sammccall accepted this revision. sammccall added a comment. Very nice, thanks! I'll land this for you now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92041/new/ https://reviews.llvm.org/D92041 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D92041: [clangd] Add hover info for `this` expr

xndcn updated this revision to Diff 311256. xndcn added a comment. Thank you, it works like a charm! For class withou template, `getHoverInfo(QualType ...)` will add namespace scope by default, so I have to add `SuppressScope` printpolicy here. CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D92041: [clangd] Add hover info for `this` expr

sammccall added a comment. Sorry, I didn't really address your question. I think the difficulty in making this a printingpolicy is that going from `type-parameter-0-0` to the spelled parameter is going from a canonical type to its alias, and isn't really possible in the general case, just in a

[PATCH] D92041: [clangd] Add hover info for `this` expr

sammccall added a comment. In D92041#2448176 , @xndcn wrote: > In D92041#2446366 , @sammccall wrote: > >> Sorry for the delay here. Kadir is out on vacation. >> >> Yikes - it's a shame reusing our existing type prin

[PATCH] D92041: [clangd] Add hover info for `this` expr

xndcn added a comment. In D92041#2446366 , @sammccall wrote: > Sorry for the delay here. Kadir is out on vacation. > > Yikes - it's a shame reusing our existing type printing doesn't do the right > thing, but injected-classname and partial specializations

[PATCH] D92041: [clangd] Add hover info for `this` expr

sammccall added a comment. Sorry for the delay here. Kadir is out on vacation. Yikes - it's a shame reusing our existing type printing doesn't do the right thing, but injected-classname and partial specializations are indeed weird. I'm tempted to say just to live with the "type-parameter-0-0" no

[PATCH] D92041: [clangd] Add hover info for `this` expr

xndcn updated this revision to Diff 308994. xndcn added a comment. `getHoverInfo(CXXThisExpr->getType()->getPointeeType(), ...)` does not output namespace scope and template parameters without specialization: F14499632: Pointee.png while `getHoverInfo(CXXThis

[PATCH] D92041: [clangd] Add hover info for `this` expr

xndcn added a comment. Thanks, it look more clear really. I'm trying to make the hover looks like `auto` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92041/new/ https://reviews.llvm.org/D92041 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D92041: [clangd] Add hover info for `this` expr

kadircet requested changes to this revision. kadircet added a comment. This revision now requires changes to proceed. In D92041#2424928 , @njames93 wrote: > One last point, is it worth including cv qualifications in the hover info? Yes, I think we should

[PATCH] D92041: [clangd] Add hover info for `this` expr

xndcn added a comment. In D92041#2424869 , @kadircet wrote: > can you give me an email address to associate the commit with? xnd...@gmail.com, thank you. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92041/new/ https://reviews.llvm.org/D92041

[PATCH] D92041: [clangd] Add hover info for `this` expr

njames93 added a comment. One last point, is it worth including cv qualifications in the hover info? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92041/new/ https://reviews.llvm.org/D92041 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D92041: [clangd] Add hover info for `this` expr

kadircet added a comment. can you give me an email address to associate the commit with? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92041/new/ https://reviews.llvm.org/D92041 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D92041: [clangd] Add hover info for `this` expr

xndcn added a comment. In D92041#2420251 , @kadircet wrote: > Do you have commit access or should I commit this for you? I still don't have commit access, so please help me commit this, thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92

[PATCH] D92041: [clangd] Add hover info for `this` expr

kadircet added a comment. Do you have commit access or should I commit this for you? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92041/new/ https://reviews.llvm.org/D92041 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://list

[PATCH] D92041: [clangd] Add hover info for `this` expr

xndcn updated this revision to Diff 307947. xndcn added a comment. Thanks. Update commit to fix the last nit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92041/new/ https://reviews.llvm.org/D92041 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/clangd/unittests/HoverT

[PATCH] D92041: [clangd] Add hover info for `this` expr

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, LGTM! Comment at: clang-tools-extra/clangd/Hover.cpp:644 +const NamedDecl *D = CTE->getType()->getPointeeType()->getAsTagDecl(); +HI = getHoverContents(D,

[PATCH] D92041: [clangd] Add hover info for `this` expr

xndcn updated this revision to Diff 307749. xndcn added a comment. Thanks! Update the commit as review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92041/new/ https://reviews.llvm.org/D92041 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/clangd/unittests/Hov

[PATCH] D92041: [clangd] Add hover info for `this` expr

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:610 +/// Generate a \p Hover object given the \p this pointer. +HoverInfo getHoverContents(const CXXThisExpr *CTE, const SymbolIndex *Index) { + const NamedDecl *D = CTE->getType()->getPointeeType()->

[PATCH] D92041: [clangd] Add hover info for `this` expr

xndcn updated this revision to Diff 307622. xndcn added a comment. Update the diff with `getHoverContents(const NamedDecl ..)` overload function. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92041/new/ https://reviews.llvm.org/D92041 Files: clang-tools-extra/clangd/Hover.cpp clang

[PATCH] D92041: [clangd] Add hover info for `this` expr

xndcn added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:610 +/// Generate a \p Hover object given the \p this pointer. +HoverInfo getHoverContents(const CXXThisExpr *CTE, const SymbolIndex *Index) { + const NamedDecl *D = CTE->getType()->getPointeeType()->get