[PATCH] D134453: Disambiguate type names when printing NTTP types

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

I think the type printer's purpose is for printing types for diagnostics, not for generating a lossless representation of types as strings, as the ast dumper does.
So packing so much information in them that hurts readability would be against that purpose.
I do agree with everyone here that the provided example is one where we are unhelpful and provide too little information.

When printing types for diagnostics we roughly want to:

1. Point the user to the relevant type which is being diagnosed, as in the source code. When we have a TypeLoc, and that would otherwise be more presentable for the given diagnostic, we can point to the type with a caret. Otherwise, we try to print the type as closely as we can to what was written, so that the user can connect the diagnostic with something that was done in source code.
2. Give some sort of explanatory analysis, revealing implicit / context sensitive information which might be relevant.

I think that for the motivating example, we completely fail to provide 1), we don't have a TypeLoc, and we only have a canonical type.
I think fixing 1) is the most pressing issue, and is not addressed by this patch.
Ideally, long term, we can remove all instances where we try to print canonical types for diagnostic purposes, and not have the type printer deal with that, simplifying things.

For the short term, we have canonical types, and we need the type printer to deal with them reasonably.
I agree that the way MSVC is presenting these canonicalized NTTPs of class type looks the most sensible here.

But by fixing 1), we have more options. We can do something like:

  no known conversion from 'NDArray<[...], W (aka 'Width{}')>' to 'const NDArray<[...], H (aka 'Height{}')>' for 1st argument

Or perhaps instead of the aka, we can generate notes which place a caret in source code for each of W and H.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134453/new/

https://reviews.llvm.org/D134453