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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D134453#3869201 <https://reviews.llvm.org/D134453#3869201>, @aaron.ballman wrote:

> In D134453#3868821 <https://reviews.llvm.org/D134453#3868821>, @DoDoENT wrote:
>
>> In D134453#3868789 <https://reviews.llvm.org/D134453#3868789>, @dblaikie wrote:
>>
>>> I still don't think "keep full NTTP type printing behind a policy, for those that want/need that" is a policy we should add. String printed names aren't meant to be a tool for type reflection - the AST can be queried for that information.
>>
>> I agree on that matter.
>>
>> However, I'm building my clang with this policy enabled by default to provide my developers with GCC/MSVC-like verbosity in diagnostic messages. They prefer it that way.
>
> @dblaikie -- but I thought we agreed that we would always print the full type, so the policy in this case is to print information *less* suitable for type reflection, right?

I'm confused - I'll try to say some things that might lead to less confusion, but I'm far from sure.

(I'm confused by the question/statement above by itself - "agreed we would always print the full type" + "print information *less* suitable for type reflection". Are those meant to go together? I think printing the "full" type (that might be ambiguous/unclear what full means in this conversation, but best guess) would make something more suitable for type reflection? (type reflection in the sense of "I can look at/analyze/split up the string and see type names that may be useful for some tooling purpose"))

My understanding is that there are 3 possible ways of printing under discussion (all demonstrated here: https://godbolt.org/z/1s8Mf6b8K ):

  struct t1 { int v1; };
  struct t2 { t1 v1; };
  template<auto>
  struct t3 { };

1. `t3<t2{t1{42}}>`: Fully explicit - all types provided. Valid code, though not strictly necessary.
2. `t3<t2{{42}}>`: Semi-explicit. Necessary for validity, even if the NTTP is not `auto`, but instead explicitly `t2` (C++ won't deduce it - requiring it to be explicit).
3. `t3<{{3}}>`: Not actually valid code (even if the NTTP is `t2`)

Clang produces (3) in some cases, and (2) in others.
MSVC and GCC always use (1).

When you say "always print the full type" - it sounds like (1)? But I thought/my preference has been that we should always use (2) because (1) can be quite verbose.

The patch I think currently makes Clang use (2) more consistently, and provides a PrintingPolicy flag to do (1).

I think the fix to use (2) more consistently is good, but I don't think we should add functionality for (1).

> However, given that we don't think we'll use that policy in the tree, can that policy can be kept in your downstream instead @DoDoENT?

That would tend to be my conclusion/preference at this point.

But open to your perspective/preferences/etc, @aaron.ballman


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134453