sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/AST.cpp:353
if (auto *AT = D->getType()->getContainedAutoType()) {
- if (!AT->getDeducedType().isNull())
- DeducedType = AT->getDeducedType();
+ DeducedType = AT->desugar();
}
----------------
I added a comment to getDeducedType() - I think the intent here is that we
return the AutoType itself if it's not deduced, right?
================
Comment at: clang-tools-extra/clangd/Hover.cpp:591
+ ASTContext &ASTCtx,
+ const SymbolIndex *Index) {
+ QualType OriginThisType = CTE->getType()->getPointeeType();
----------------
I dropped this unused param
================
Comment at: clang-tools-extra/clangd/Hover.cpp:618
+ } else {
+ CXXRecordDecl *D = QT->getAsCXXRecordDecl();
+ if (D && D->isLambda())
----------------
sammccall wrote:
> qchateau wrote:
> > sammccall wrote:
> > > You've rewritten this logic compared to the old
> > > `getHoverContents(QualType)`, and I think there are regressions:
> > > - We've dropped class documentation (see e.g.
> > > ac3f9e48421712168884d59cbfe8b294dd76a19b, this is visible in the tests)
> > > - we're no longer using printName() to print tag-decls, which I expect
> > > changes e.g. the printing of anyonymous structs which is special-cased in
> > > that function (we have tests for this but probably not in combination
> > > with auto)
> > > - we're no longer using the printing-policy to print non-tag types,
> > > which will lead to some random changes
> > >
> > > I don't see a reason for these changes, can we revert them?
> > - I can re-add class documentation, but should it work when `auto` is a
> > pointer or a ref ? In that case, I'll need something like your `unwrapType`
> > of https://reviews.llvm.org/D93314
> > - `printName` will print `C` instead of `class C` even if I hack around and
> > give it the right `PrintingPolicy`. The problem is that IDE (at least
> > VSCode) does not know what `C` is and cannot color it, which looks a nice
> > less nice. Up to you !
> > - I can re-add the printing policy for non-tag types, and add it for
> > tag-types if we chose not to use `printName`.
> > should it work when auto is a pointer or a ref?
>
> I think no need for now, I just want to avoid the regression.
> (Nice to have, but at least in the codebases I work in, `auto` is much more
> common than `decltype(auto)` so it's rarely a reference, and pointers are
> idiomatically written `auto*`)
>
> > printName will print C instead of class C even if I hack around and give it
> > the right PrintingPolicy.
>
> Yeah... it's not supposed to, though the extra clarity is valuable here. You
> could just hack around this and prepend the TagTypeKind yourself :-)
Hmm, it looks like this wasn't done. It's now printing e.g. `class vector<class
Foo>` which seems more confusing than either `class vector<Foo>` or
`vector<Foo>` (even if we lose some highlighting). I'll send a followup with
proposed changes here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93227/new/
https://reviews.llvm.org/D93227
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
