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.

Unit tests: pass. 61742 tests passed, 0 failed and 780 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

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?

Harbormaster completed remote builds in B43669: Diff 237284.Jan 10 2020, 4:33 AM

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 could reasonably be done I guess is TypePrinter, because we can't actually transform the types into the correct form IIUC.
It's particularly unclear to me why typeprinter descends into auto but prints decltype, but Kadir says that seems to be intentional.

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?

Canonical types are often *really* ugly, especially with STL types (we don't have the "as written" form). And presenting the types twice might be at least as confusing/noisy as helpful. But if you have examples where this would be better, it'd be interesting.

In D72498#1813899, @ilya-biryukov wrote:

This does not work for more complicated types, though?
E.g. decltype(a+b)* a or vector<decltype(a+b)> a?

yes, and I think we should have a more sophisticated way to print composite types including decltypes to cover more cases.

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

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 agree with your suggestion for typedefs though, I think there would be value in displaying the underlying type in hover card for type aliases to reduce navigation.

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 inconsistency and I haven't seen any indication it's not a coincidence.
@kadircet, why do you think it's intentional? Should we put some comments there?

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 here. decltype is normally the last resort, so whatever it produces is probably super-obscure, would even expect it to be not representable in C++ in many cases.
E.g.

auto Callback = []() { ... };
decltype(Callback) ^a = Callback;

Typedefs are often used with simple types, so that's not necessarily the case.

typedef unordered_map<int, int> IntMap;
IntMap ^a =;

I agree with your suggestion for typedefs though, I think there would be value in displaying the underlying type in hover card for type aliases to reduce navigation.

Would definitely be helpful. If you feel we have some room in hover, I would love to have that. But there's a balance to be made, see Sam's comments about canonical types being obscure. I agree on 50% of the cases :-)

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 presenting the types twice might be at least as confusing/noisy as helpful. But if you have examples where this would be better, it'd be interesting.

I'm mostly trying to find a consistent rule we can apply to make these decision.
auto, decltype and typedefs are very similar in this regard, it's a bit confusing we use different rules for those. Although I can see how auto and decltype could easily be perceived differently and fall into a different group.

STL types are unfortunate, that is so true. And most other types aren't. It could be worth looking into the rules clang applies in diagnostics (have you seen those that say type X(aka vector<int>)?
I have no easy answer, though, it feels this will inevitably lead to terrible presentations of some types. FWIW, showing underlying type of decltype could lead us there too.

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 more mixed.

The only thing that gives me pause is that it does seem to be decltype and auto belong in the same bucket (more so than typedef). If we can make those consistent in typeprinter, I'm happy.

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.
I think it's worth fixing in this case.

template <typename T1, typename T2>
auto sum(const T1 &t1, const T2 &t2) -> decltype(t1 + t2) {
  return t1 + t2;
}

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 minimal repo.
I think it's worth fixing in this case.
template<typename T1, typename T2>  
auto sum(T1 &t1, T2 &t2) ->decltype(t1 + t2))
{  
	return t1 + t2;  
}

Thanks for the clearer example! I agree that's one where it'd be nice to show the result type and hovering over decltype won't give it to you.

But note that this patch doesn't handle the important use case of decltype as return type, really!
imagine cout << sum('c', 4), and you want to know what type the function call has.
The natural thing is to hover over the function call, you'll see "function sum<char, int> -> decltype(t1 + t2)" which is indeed pretty useless.
But this patch doesn't fix that behaviour, it only handles types of declarations.

@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 this idiom?

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.

class Bar {
private:
  struct Inner {
    operator bool();
  };

public:
  Inner &operator()(int a, int b);
};

template <typename Func, typename... Args>
auto call(Func &&func, Args &&... args)
    -> decltype(func(std::forward<Args>(args)...));

template <typename T> void useRes(T &&t);

void foo() {
  Bar bar;
  // Under c++11 we don't have decltype(auto), using auto here will lose
  // reference.
  decltype(call(bar, 5, 6)) res = call(bar, 5, 6);
  if (res) {
    // long code to process res
    useRes(res); // User wants to know the type of res here.
  } else {
    // long code to process res
    useRes(res);
  }
}

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 helpful to show the canonical type in this case.

int main() {
  std::vector<int> a;
  a.front();
  a.push_back(5);
}

hover over the front , you'll see "instance-method front → std::vector<int, class std::allocator<int> >::reference".
hover over the push_back, you'll see "std::vector<int, class std::allocator<int> >::value_type && __x".

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 this idiom?

If we feel it's useful in the function return type, it's probably also useful in other template contexts:
E.g.

template <class T>
struct X {
  typedef decltype(T() + T()) add_result_type;
};

X<int>::^add_result_type y;

And I don't think it's used in practice in more contexts.

Moreover, I believe usages in function returns will become more rare as projects move to C++14 and beyond (auto return type gives the same results in most interesting cases without code duplication).

Therefore, I'd not special-case for function return types.

In D72498#1815500, @lh123 wrote:

hover over the front , you'll see "instance-method front → std::vector<int, class std::allocator<int> >::reference".

hover over the push_back, you'll see "std::vector<int, class std::allocator<int> >::value_type && __x".

These look terrible and are the great examples where showing canonical types results in better output than canonical types.
I wonder why we add std::vector<int, class std::allocator<int>>:: in the first place, I believe the standard library uses value_type in the declaration. Showing value_type is not great, but at least that doesn't uglify what was written in the code in the first place.
FWIW, I think the perfect output in those cases would be int (aka value_type)

In D72498#1816244, @ilya-biryukov wrote:

In D72498#1815500, @lh123 wrote:

hover over the front , you'll see "instance-method front → std::vector<int, class std::allocator<int> >::reference".

hover over the push_back, you'll see "std::vector<int, class std::allocator<int> >::value_type && __x".

These look terrible and are the great examples where showing canonical types results in better output than canonical types.
I wonder why we add std::vector<int, class std::allocator<int>>:: in the first place, I believe the standard library uses value_type in the declaration. Showing value_type is not great, but at least that doesn't uglify what was written in the code in the first place.
FWIW, I think the perfect output in those cases would be int (aka value_type)

Indeed. Another illustrative example, the return type of vector<int64_t>::at() - we'd probably want int64& here, rather than vector<...>::reference or unsigned long long/unsigned long depending on platform.

It seems like:

unwrapping nothing isn't ideal
unwrapping everything isn't ideal
brevity might be a good heuristic, but unclear
there's value sometimes in showing multiple forms, unclear exactly when

(Machine learning time? Mostly joking...)

In D72498#1816339, @sammccall wrote:

In D72498#1816244, @ilya-biryukov wrote:

In D72498#1815500, @lh123 wrote:

hover over the front , you'll see "instance-method front → std::vector<int, class std::allocator<int> >::reference".

hover over the push_back, you'll see "std::vector<int, class std::allocator<int> >::value_type && __x".

These look terrible and are the great examples where showing canonical types results in better output than canonical types.
I wonder why we add std::vector<int, class std::allocator<int>>:: in the first place, I believe the standard library uses value_type in the declaration. Showing value_type is not great, but at least that doesn't uglify what was written in the code in the first place.
FWIW, I think the perfect output in those cases would be int (aka value_type)

Indeed. Another illustrative example, the return type of vector<int64_t>::at() - we'd probably want int64& here, rather than vector<...>::reference or unsigned long long/unsigned long depending on platform.

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://github.com/clangd/clangd/issues/58#issuecomment-507800970

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 Kadir says that seems to be intentional.

Also don't see why they choose to have this inconsistency and I haven't seen any indication it's not a coincidence.
@kadircet, why do you think it's intentional? Should we put some comments there?

I was saying that because TypePrinter deliberately prints decltype( and then the expression.
I suppose we could make this a printing policy option to print either the underlying expr or type.

I tend to disagree here. decltype is normally the last resort, so whatever it produces is probably super-obscure, would even expect it to be not representable in C++ in many cases.

I was rather talking about the obscurity of the expression inside decltype vs the typedef alias. I believe it is a lot harder to make any assumptions on decltype(callback) compared to IntMap without seeing the underlying type.

Would definitely be helpful. If you feel we have some room in hover, I would love to have that. But there's a balance to be made, see Sam's comments about canonical types being obscure. I agree on 50% of the cases :-)

I think this should be OK to spend some space, as it will only show up when needed. I believe better printing of canonical types is a different problem we should definitely solve.

@sammccall wrote:

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?).

Right, the user can definitely hover over the decltype, but this goes against our objectives. I believe with hover we are trying to reduce number of jumps a user needs to take for acquiring some info,
and requiring them to jump to the definition of symbol for figuring out the type doesn't seem right.

But note that this patch doesn't handle the important use case of decltype as return type, really!
imagine cout << sum('c', 4), and you want to know what type the function call has.
The natural thing is to hover over the function call, you'll see "function sum<char, int> -> decltype(t1 + t2)" which is indeed pretty useless.
But this patch doesn't fix that behaviour, it only handles types of declarations.

Yes, but this was an oversight rather than an explicit decision. I believe we should definitely do the same for return type and even parameter types.

@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 this idiom?

I wouldn't special case that behavior as it is equally useful for ValueDecls, since we are trying to get rid of the extra jump as mentioned above.

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://github.com/clangd/clangd/issues/58#issuecomment-507800970

That actually doesn't look bad. Maybe let's try doing that and see whether we'll get negative feedback?
That seems to give useful information in all cases, so at least it'll cover all use-cases even it's more verbose.

What do others think?

In D72498#1816668, @kadircet wrote:

In D72498#1813989, @ilya-biryukov wrote:

In D72498#1813962, @sammccall wrote:

I tend to disagree here. decltype is normally the last resort, so whatever it produces is probably super-obscure, would even expect it to be not representable in C++ in many cases.

I was rather talking about the obscurity of the expression inside decltype vs the typedef alias. I believe it is a lot harder to make any assumptions on decltype(callback) compared to IntMap without seeing the underlying type.

Point taken, although I bet we could come up with examples of obscure results in both cases.

Would definitely be helpful. If you feel we have some room in hover, I would love to have that. But there's a balance to be made, see Sam's comments about canonical types being obscure. I agree on 50% of the cases :-)

I think this should be OK to spend some space, as it will only show up when needed. I believe better printing of canonical types is a different problem we should definitely solve.

Totally agree, improving printing of STL types would be huge, no matter whether they're canonical or not.

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

This has been used in ycmd for ~4 years without complaint. https://github.com/clangd/clangd/issues/58#issuecomment-507800970

That actually doesn't look bad. Maybe let's try doing that and see whether we'll get negative feedback?
That seems to give useful information in all cases, so at least it'll cover all use-cases even it's more verbose.

What do others think?

SGTM, happy to update all types in HoverInfo to contain both a pretty-printed and canonical version. Where pretty-printed would just means desugared, so keywords like auto/decltype would've
been stripped away and it would be the type as written in all other cases, while canonical would refer to clang canonical with all of the type aliases etc. resolved. Ofc.
Does that SG to you as well @sammccall ?

In D72498#1816957, @kadircet wrote:

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

This has been used in ycmd for ~4 years without complaint. https://github.com/clangd/clangd/issues/58#issuecomment-507800970

That actually doesn't look bad. Maybe let's try doing that and see whether we'll get negative feedback?
That seems to give useful information in all cases, so at least it'll cover all use-cases even it's more verbose.

What do others think?

SGTM, happy to update all types in HoverInfo to contain both a pretty-printed and canonical version. Where pretty-printed would just means desugared, so keywords like auto/decltype would've
been stripped away and it would be the type as written in all other cases, while canonical would refer to clang canonical with all of the type aliases etc. resolved. Ofc.
Does that SG to you as well @sammccall ?

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 behavior and presentation for this cycle. I think we should do something more conservative and then experiment in the next release cycle.

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 behavior and presentation for this cycle. I think we should do something more conservative and then experiment in the next release cycle.

Sorry, I didn't mention it explicitly in my previous post but I was also planning to make it a separate patch, independent of this one. I am also OK with landing it after branch cut.

As for this patch, I still believe there's a lot of value in either:

handling non-composite type cases, as proposed in current change
handling of both basic and composite type cases in typeprinter through a printing policy

I am happy to follow any of those, though the second might be more disruptive before the branch cut.
So I would rather also introduce handling of decltypes to parameter and return types and land this change.
Because even though it might be messy to hover over nested templated types, I still believe it will most likely save user time in the general case.