[PATCH] D130894: [clang] Print more information about failed static assertions

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb accepted this revision.
cjdb added a comment.
This revision is now accepted and ready to land.

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

> In D130894#3709027 <https://reviews.llvm.org/D130894#3709027>, @tbaeder wrote:
>
>> I don't really want to bike-shed about the presentation for too long...
>
> I understand the desire, but at the same time, the whole goal of this patch is to improve the presentation of the diagnostic. You can't invite us all to a bikeshed painting party and then ask us not to use our brushes, that's just cruel! :-D

Agreed. Getting the presentation of diagnostics right is critical.

>> I'm fine with just removing the parens, since we present it like that in the error message as well anyway:
>>
>>   ./assert.cpp:6:1: error: static assertion failed due to requirement ''c' == 'a''
>>   static_assert('c' == 'a')
>
> That's still my preference as well. Splitting across multiple lines with hard line breaks has some issues for some IDEs that want to display diagnostic information in a listbox (IIRC), so I'd rather we avoid ad hoc use of line breaks in diagnostics for the moment (it'd be easier/more appropriate if we were making a diagnostic policy for using multiple lines, but that'd involve an RFC and is more effort than I think is needed for this patch).

Fair enough, no further objections on my part.

>>> Any tests with macros?
>>
>> I can add some, but they should be handled transparently, with the usual "expanded from macro" note.
>
> I'm fine either way.




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

https://reviews.llvm.org/D130894