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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

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

>> it's up to IDE's and good editors and CI log parsers to parse that and present in a possibly better format.
>
> Lots of folks use the diagnostic info from their compiler without IDEs or log parsers doing anything else to them - we should be trying to design these things for usability directly. (SARIF and such are probably the direction to go when producing diagnostics intended to be massaged by tooling in various ways - could provide collapsed views of template type names or these struct literals - comparitive highlighting views (though we do some of that for template type diffing in clang today - maybe one day we'll do that/expand it to cover the struct literals too))

+1 to the point that we want to design our diagnostics to be usable without requiring another tool, and +1 to using SARIF for when we want to pass our diagnostics to another tool which modifies them somehow.

>> So we might print it with a syntax as if the class was a simple pod type, even though that might produce a completely different result if used in practice.
>> I don't think we can do much better without having the actual expression used.
>>
>> That might be a point in favor of the designated-initializer style with the field names, that should never produce the wrong result if copy-pasted back to code.
>
> Oh, yeah, I hadn't considered that. :/ that is awkward & something I would certainly place some value in avoiding (was one of the issues I was motivated by in the earlier discussion, that it'd be useful if we produced an identifier that also happened to be valid code - but I wasn't considering the case where we might produce something valid-but-incorrect, just valid being better than invalid - but yeah, invalid being better than valid-but-incorrect).

Yeah, I would place value in avoiding that outcome as well. Also, the point by @mizvekov that the types may be internal types is kind of compelling to me as well; I was thinking that the types involved are most often going to be ones users are aware of, but that's certainly not true for highly generic code like STL implementations.

In the interests of getting this review unstuck, I think we should 1) remove the printing policy option (which should address the concerns from @dblaikie about use of that policy for introspection purposes); the policy is something that @DoDoENT can carry in their downstream, 2) keep the changes in TemplateBase.cpp and drop the changes from APValue.cpp.

That gives us an incremental improvement over what the status quo is, and we can debate other improvements (if we can think of any we'd like to make) separately. Is everyone comfortable with this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134453