Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Enhance type deduce on Hover. #58

Open
lh123 opened this issue Jun 12, 2019 · 4 comments
Open

[Feature Request] Enhance type deduce on Hover. #58

lh123 opened this issue Jun 12, 2019 · 4 comments
Assignees

Comments

@lh123
Copy link

@lh123 lh123 commented Jun 12, 2019

Expected

1. Hover on decltype
1

2. Hover on template type
2
3

Current

1. Hover on decltype
4

2. Hover on template type
5
6

@lh123 lh123 changed the title [Feature Request] Enhance Hover on type deduce. [Feature Request] Enhance type deduce on Hover. Jun 12, 2019
@sam-mccall

This comment has been minimized.

Copy link
Contributor

@sam-mccall sam-mccall commented Jun 12, 2019

Agree with these (though not sure the "expected" behavior is ideal, I think we just need to show both in some cases).

Current implementation is a couple of special cases:

  • hover on a decltype or auto keyword shows its expansion
  • hover on a reference to an expr shows its declaration (mostly as spelled, with initializer)

Showing actual vs spelled types is a tricky tradeoff. Expanding auto is usually good (not always possible). Expanding typedefs is iffier.

I think the plan of record is to show both the (expanded) type, and the spelled initializer.

e.g.


Variable a
Declared in function main

Type: int
Value: 0

TestHover<int>::Type a;


@kadircet similar to adding values of constants, we may have to modify the current approach of "resolve everything to a Decl and then extract", in favor of extracting some info from the Expr, TypeLoc etc.

e.g. in the second example here, the typeloc TestHover<int>::Type spells a concrete type int, where the decl Type it resolves to is dependent. So the correct Type information can only be extracted at the TypeLoc level.

There's probably two stages of heuristic needed: the first one to determine "what's the interesting thing under the cursor (declaration, expression, typeloc, stmt)", and then the second to extract information (e.g. by traversing Expr->DeclRefExpr->Decl. SelectionTree can help with the former I think.

@bstaletic

This comment has been minimized.

Copy link

@bstaletic bstaletic commented Jul 2, 2019

Isn't this basically a duplicate of #36?

@puremourning

This comment has been minimized.

Copy link

@puremourning puremourning commented Jul 2, 2019

Showing actual vs spelled types is a tricky tradeoff. Expanding auto is usually good (not always possible). Expanding typedefs is iffier.

Well, I added the GetType request to ycmd specifically for this use case. Our decision was to always show both the expanded (canonical) type and the referenced type, if they differ. This has been used in ycmd for ~4 years without complaint. Not expanding auto to the deduced type is pretty bad, as one of the key things that "modern c++" zealots will tell you is that "your tools can tell you the type if you need to know it".

Here are some test cases: https://github.com/ycm-core/ycmd/blob/master/ycmd/tests/clang/subcommands_test.py#L479-L530

@kadircet

This comment has been minimized.

Copy link

@kadircet kadircet commented Jan 15, 2020

there have been some changes to hover, the current state is:

  • prefer the type as written
  • print the canonical type when the type is not spelled in the source code(auto, decltype)
  • use template instantiations rather than patterns, so we display using Type = int not T.
  • don't provide underlying type for decltypes and type aliases(except the one in definition line)
  • provide underlying canonical type for auto
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.