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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D134453#3869610 <https://reviews.llvm.org/D134453#3869610>, @dblaikie wrote:

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

Ah, and I thought/my preference was that we should always use (1) because (2) requires the reader to infer information that may not be obvious from local context.

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

I was thinking we should use (1) consistently and not add the functionality for (2). Huttah, we found the confusion! :-D

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

I got my perspective, perhaps incorrectly, from this bit of the conversation:

In D134453#3834659 <https://reviews.llvm.org/D134453#3834659>, @dblaikie wrote:

> Ok, so sounds like we're on the same page that `t1<{}>` is a diagnostic quality bug and we'd love to see a fix for it to include the top level type of the NTTP, as in `t1<t2{}>`, and that shouldn't need a new printing policy - because we never want to print `t1<{}>` and anywhere we do is a bug to be fixed.

I think my confusion came from `<{}>` -- I was interpreting that as meaning any braces without a type preceding them, which `t2{}` would resolve. I think you intended me to read that as empty braces without a preceding type, specifically.

I can sympathize with the argument that brevity can be good thing, but I think that biases towards expert users over novice users of the language in terms of making the information understandable, at least in some small way. I tend to come down on the side of thinking verbosity is better than terseness for non-expert users because I think it's easier for humans to ignore information they think is irrelevant compared to correctly inferring information where it's missing, but I can also see the argument that you can overwhelm people with too much information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134453