[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/

[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

[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:

[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

[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

[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

[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. > >

[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

[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

[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 &&

[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

[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

[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 (int a, int b); template auto

[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

[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 , T2 ) ->decltype(t1 + t2)) { return

[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

[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

[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

[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

[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

[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/

[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/

[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

[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