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:
> what about using the existing `getHoverContents(QualType ..)` overload
> instead ?
> what about using the existing `getHoverContents(QualType ..)` overload
> instead ?
================
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:
> 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.
================
Comment at: clang-tools-extra/clangd/Hover.cpp:644
// FIXME: Support hover for literals (esp user-defined)
llvm::Optional<HoverInfo> getHoverContents(const Expr *E, ParsedAST &AST) {
// There's not much value in hovering over "42" and getting a hover card
----------------
kadircet wrote:
> can you handle `CXXThisExpr` inside this function, instead of an extra branch
> in the `getHover`?
Thanks, here need additional parameter `Index *`, will update patch soon.
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
