[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

This revision was automatically updated to reflect the committed changes. Closed by commit rG0474fe465d8f: [clangd] Print underlying type for decltypes in hover (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72498/new/ https:/

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

sammccall accepted this revision. sammccall added a comment. Still LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72498/new/ https://reviews.llvm.org/D72498 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61913 tests passed, 0 failed and 783 were skipped. {icon question-circle color=gray} clang-tidy: unknown. {icon check-circle color=green} clang-format: pass. Build artifacts

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

kadircet updated this revision to Diff 238474. kadircet added a comment. - Handle return and parameter types as well Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72498/new/ https://reviews.llvm.org/D72498 Files: clang-tools-extra/clangd/Hover.c

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61885 tests passed, 0 failed and 782 were skipped. {icon question-circle color=gray} clang-tidy: unknown. {icon check-circle color=green} clang-format: pass. Build artifacts

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

kadircet updated this revision to Diff 238245. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72498/new/ https://reviews.llvm.org/D72498 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/clangd/unitt

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

kadircet added a comment. In D72498#1817070 , @sammccall wrote: > No, I think printing *both* is at least somewhat likely to be too verbose, > especially since the previous release showed no types at all. > And we're out of time to iterate on the behavio

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

sammccall added a comment. In D72498#1816957 , @kadircet wrote: > In D72498#1816785 , @ilya-biryukov > wrote: > > > In D72498#1816424 , @lh123 wrote: > > > > > Currently, I

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

kadircet added a comment. In D72498#1816785 , @ilya-biryukov wrote: > In D72498#1816424 , @lh123 wrote: > > > Currently, I think that in most cases, showing both expanded (canonical) > > and spelled types is suffi

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. In D72498#1816424 , @lh123 wrote: > Currently, I think that in most cases, showing both expanded (canonical) and > spelled types is sufficient. > > > This has been used in ycmd for ~4 years without complaint. > > https://gi

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

kadircet added a comment. In D72498#1813989 , @ilya-biryukov wrote: > In D72498#1813962 , @sammccall wrote: > > > It's particularly unclear to me why typeprinter descends into auto but > > prints decltype, but Kad

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

lh123 added a comment. In D72498#1816339 , @sammccall wrote: > In D72498#1816244 , @ilya-biryukov > wrote: > > > In D72498#1815500 , @lh123 wrote: > > > > > - hover over the

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

sammccall added a comment. In D72498#1816244 , @ilya-biryukov wrote: > In D72498#1815500 , @lh123 wrote: > > > - hover over the `front` , you'll see "instance-method `front` → > > `std::vector >::reference`". > >

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. In D72498#1815500 , @lh123 wrote: > - hover over the `front` , you'll see "instance-method `front` → > `std::vector >::reference`". > - hover over the `push_back`, you'll see "`std::vector std::allocator >::value_type && __x

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. In D72498#1814366 , @sammccall wrote: > @ilya-biryukov @kadircet what do you think about unwrapping decltype only > when it's a return value (optional: of a function whose leading return type > is auto) to narrowly catch th

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

lh123 added a comment. In D72498#1813902 , @ilya-biryukov wrote: > Could it be the case that we want to show the canonical types (i.e. without > all syntax sugar)? > Maybe we want both the normal type and the canonical type? +1,I think it would be hel

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

lh123 added a comment. > what do you think about unwrapping decltype only when it's a return value > (optional: of a function whose leading return type is auto) to narrowly catch > this idiom? I think it's worth fixing in the declarations too. int &bar(int a, int b); template auto ca

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

sammccall added a comment. In D72498#1814286 , @lh123 wrote: > In D72498#1814008 , @sammccall wrote: > > > I think i'm also comfortable with marking the linked bug as wontfix. > > > The previous example is just mini

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

lh123 added a comment. In D72498#1814008 , @sammccall wrote: > I think i'm also comfortable with marking the linked bug as wontfix. The previous example is just minimal repo. template auto sum(T1 &t1, T2 &t2) ->decltype(t1 + t2)) { ret

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

sammccall added a comment. I think i'm also comfortable with marking the linked bug as wontfix. It's a contrived example that makes clangd look silly (why decltype(a) instead of int?) but also the user look silly (why hover the variable rather than the decltype?). Real examples are certainly mo

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. In D72498#1813962 , @sammccall wrote: > Maybe we want both the normal type and the canonical type? > > Canonical types are often *really* ugly, especially with STL types (we don't > have the "as written" form). And presentin

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. In D72498#1813963 , @kadircet wrote: > I think typedef and decltype have different nature, the latter is a lot more > obscure than the former, that was the reason why I handled decltypes > specifically. I tend to disagree

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. In D72498#1813962 , @sammccall wrote: > It's particularly unclear to me why typeprinter descends into auto but prints > decltype, but Kadir says that seems to be intentional. Also don't see why they choose to have this inc

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

kadircet marked an inline comment as not done. kadircet added a comment. In D72498#1813899 , @ilya-biryukov wrote: > This does not work for more complicated types, though? > E.g. `decltype(a+b)* a` or `vector a`? yes, and I think we should have a more

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

sammccall added a comment. Yeah we discussed that case offline and I should have mentioned. Resolving this in places other than the top-level would be nice and probably worthy of a comment at least. But this special case seems common enough to be worth having. The only place fully resolving coul

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61742 tests passed, 0 failed and 780 were skipped. {icon check-circle color=green} clang-tidy: pass. {icon check-circle color=green} clang-format: pass. Build artifacts

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. Could it be the case that we want to show the canonical types (i.e. without all syntax sugar)? Maybe we want both the normal type and the canonical type? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72498/new/ https

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

ilya-biryukov added a comment. This does not work for more complicated types, though? E.g. `decltype(a+b)* a` or `vector a`? Why do we prefer to drop `decltype()`, yet show the typedefs? Both could lead to complicated types, arguably decltypes can be even worse than typedefs. Repository: rG

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

kadircet updated this revision to Diff 237284. kadircet marked an inline comment as done. kadircet added a comment. - Update comment - Iteratively resolve underlying decltypes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72498/new/ https://review

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61742 tests passed, 0 failed and 780 were skipped. {icon check-circle color=green} clang-tidy: pass. {icon check-circle color=green} clang-format: pass. Build artifacts

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/Hover.cpp:351 +// TypePrinter doesn't resolve decltypes, so resolve them here. We are going +// to include decltype(X)

[PATCH] D72498: [clangd] Print underlying type for decltypes in hover

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Fixes https://github.com/clangd/clangd/issues/249 Repository: rG LLVM Github Monorepo https://revi