[PATCH] D92041: Add hover info for `this` expr
Kadir Cetinkaya via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 25 00:14:21 PST 2020
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()->getAsCXXRecordDecl();
----------------
xndcn wrote:
> xndcn wrote:
> > kadircet wrote:
> > > what about using the existing `getHoverContents(QualType ..)` overload instead ?
> > > what about using the existing `getHoverContents(QualType ..)` overload instead ?
> >
> >
> Thanks, I tried using `getHoverContents(QualType ..)` overload and the result looks more simplicity:
> {F14312230}
>
> The origin patch HoverInfo looks like:
> {F14312245}
>
> Both seems reasonable, not sure which one is better.
I am not sure if there's much value in repeating `this` and `type definition`. So I would go with using the `QualType` overload here.
But, rather than providing `CTE->getType()` directly, i believe we should display information for the `PointeeType`, as it will also display comments and such for `TagDecls`, rather than just providing the type-name.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92041/new/
https://reviews.llvm.org/D92041
More information about the cfe-commits
mailing list