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()->getAsCXXRecordDecl(); ---------------- kadircet wrote: > 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. Thanks! It looks more simplicity with `PointeeType`. Shall we keep the additional information like namespace and template parameters? So we can use `getHoverInfo(const NamedDecl ..)` overload? {F14313888} Repository: rG LLVM Github Monorepo 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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits