aaron.ballman added a comment.

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

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

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

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

Reply via email to