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

Reply via email to