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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to