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

Thu Oct 20 08:14:07 PDT 2022

aaron.ballman added a comment.

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

>> In terms of next steps, I think we should try to see if there's a "clear" path forward in terms of printing the types vs not printing the types. If we find one, then great, we go that way. But if we don't seem to have a clear path forward (relatively quickly; I don't think we want to drag this discussion on for months trying to find the perfect answer), then I think I'm fine with the patch as-is. It fixes the issue of `t<{}>` (with empty braces specifically) while retaining the status quo in other areas, but still exposes useful functionality through the additional printing policy. Does that sound reasonable?
>
> If you reckon (1) is better overall anyway - happy enough to defer to your opinion there and go with that, skip/omit the printing policy.

Okay, thanks for that! I'm still happy to consider alternatives if we can think of any, FWIW.

> I think the policy is like adding off-by-default warnings, and supporting a use case (using the string name for reflection when we'd recommend the AST) I don't think we want to encourage/support.

That's a fair point. So if we find no better approach, drop the policy and just always print types.

> Admittedly going with (1) means that @DoDoENT's use case will then work, until/unless we come back around and make more strategic use of type names in this printing in the future to bring down the verbosity - so I'd still discourage that use case & warn that this isn't a guarantee that all type names will be included going forward.

Absolutely agreed! IMO, whatever strings you get out of something with a printing policy generally have very little/no stability guarantees (it's part of the C++ API surface, so it's exactly as stable as any other C++ API). We'll change printing behavior whenever we feel the need to.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134453