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

aaron.ballman added a comment. In D130894#3715884 , @xbolva00 wrote: > Btw, > > https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1697r0.html Yup, that paper is currently closed in the WG21 paper tracker FWIW. Repository: rG LLVM Github Monor

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

xbolva00 added a comment. Btw, https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1697r0.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 ___ cfe-co

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

aaron.ballman added a comment. In D130894#3715725 , @mstorsjo wrote: > In D130894#3715709 , @aaron.ballman > wrote: > >> In D130894#3715143 , @xbolva00 >> wrote: >> >>>

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

mstorsjo added a comment. In D130894#3715709 , @aaron.ballman wrote: > In D130894#3715143 , @xbolva00 > wrote: > >> In D130894#3715124 , @mstorsjo >> wrote: >> >>> This

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

aaron.ballman added a comment. In D130894#3715143 , @xbolva00 wrote: > In D130894#3715124 , @mstorsjo > wrote: > >> This broke building with GCC (also noted by buildbot mails): >> >> ../tools/clang/lib/Sema/Sem

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

xbolva00 added a comment. In D130894#3715124 , @mstorsjo wrote: > This broke building with GCC (also noted by buildbot mails): > > ../tools/clang/lib/Sema/SemaDeclCXX.cpp: In member function ‘void > clang::Sema::DiagnoseStaticAssertDetails(const clang:

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

mstorsjo added a comment. This broke building with GCC (also noted by buildbot mails): ../tools/clang/lib/Sema/SemaDeclCXX.cpp: In member function ‘void clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)’: ../tools/clang/lib/Sema/SemaDeclCXX.cpp:1:19: error: declaration of ‘co

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG09117b21890c: [clang][sema] Print more information about failed static assertions (authored by tbaeder). Changed prior to commit: https://reviews.llvm.org/D130894?vs=451143&id=451744#toc Repository:

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

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, thank you for working on this, it's a great usability enhancement! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 ___ cfe-c

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

cjdb accepted this revision. cjdb added a comment. This revision is now accepted and ready to land. In D130894#3709431 , @aaron.ballman wrote: > In D130894#3709027 , @tbaeder wrote: > >> I don't really want to bi

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

tbaeder updated this revision to Diff 451143. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaDeclCXX.

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

aaron.ballman added a comment. In 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 t

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

tbaeder added a comment. I don't really want to bike-shed about the presentation for too long... 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'' stati

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

cjdb added a comment. Thank you so much for working on this! It's been on my todo list for a while and just haven't gotten around to it. In D130894#3702166 , @aaron.ballman wrote: > In D130894#3701749 , @tbaede

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

xbolva00 added a comment. Any tests with macros? 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/listinf

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

aaron.ballman added a comment. In D130894#3702233 , @tbaeder wrote: > I don't know much about fixit hints, would they be appropriate to display it > below the line itself, e.g. > > test.cpp:24:17: note: expression evaluates to > static_assert(c != c)

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

tbaeder added a comment. I don't know much about fixit hints, would they be appropriate to display it below the line itself, e.g. test.cpp:24:17: note: expression evaluates to static_assert(c != c); '0' != '0' ~~^~~~ Or would it be even better to just inline th

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

xbolva00 added a comment. or test.cpp:25:17: note: evaluated expression: '0' > 'a' 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://l

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

aaron.ballman added reviewers: erichkeane, cjdb, clang-language-wg, jyknight. aaron.ballman added a comment. In D130894#3701749 , @tbaeder wrote: > For the record, the output now looks something like this: > > test.cpp:24:1: error: static assertion fail

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

xbolva00 added a comment. looks great 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-commi

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

tbaeder added a comment. For the record, the output now looks something like this: test.cpp:24:1: error: static assertion failed due to requirement 'c != c' static_assert(c != c); ^ ~~ test.cpp:24:17: note: expression evaluates to '('0' != '0')' static_assert(c != c);

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

tbaeder updated this revision to Diff 449942. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaDeclCXX.

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

xbolva00 added a comment. personally I dont see huge value to print “ left-hand side of operator '==' evaluates to …” I would like to see the full expression. +1 to just copy gcc’s approach CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 __

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

tbaeder added a comment. IMO it's clear enough... test.cpp:24:1: error: static assertion failed due to requirement 'c != c' static_assert(c != c); ^ ~~ test.cpp:24:17: note: expression evaluates to 'a != a' static_assert(c != c); ~~^~~~ test.cpp:25:1: e

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

aaron.ballman added a comment. In D130894#3698552 , @tbaeder wrote: >> +1 to the suggestion to use quotes for a bit of visual distinction between >> the diagnostic message and the code embedded within it. > > One problem is that both the integer value `0

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

tbaeder updated this revision to Diff 449885. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaDeclCXX.

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

tbaeder updated this revision to Diff 449874. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaDeclCXX.

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

tbaeder marked 4 inline comments as done. tbaeder added a comment. > +1 to the suggestion to use quotes for a bit of visual distinction between > the diagnostic message and the code embedded within it. One problem is that both the integer value `0` and the character constant `'0'` are printed a

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

aaron.ballman added a comment. In D130894#3696590 , @xbolva00 wrote: > Use ‘5 ==6’ ? So add quotes .. +1 to the suggestion to use quotes for a bit of visual distinction between the diagnostic message and the code embedded within it.

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

xbolva00 added a comment. In D130894#3696534 , @tbaeder wrote: > FWIW, complex numbers are already covered it seems: > > test.cpp:35:1: error: static assertion failed due to requirement '__real c > == __imag c' > static_assert(__real c == __imag c);

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

tbaeder updated this revision to Diff 449669. tbaeder added a comment. Ignore what I said, they are supported now though. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/Diagnost

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

tbaeder added a comment. FWIW, complex numbers are already covered it seems: test.cpp:35:1: error: static assertion failed due to requirement '__real c == __imag c' static_assert(__real c == __imag c); ^ test.cpp:35:24: note: expression evaluates to 5 ==

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

tbaeder updated this revision to Diff 449639. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaDeclCXX.

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

tbaeder updated this revision to Diff 449637. tbaeder added a comment. Also print `nullptr` expressions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td

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

tbaeder updated this revision to Diff 449625. tbaeder added a comment. Whatever, I had time and you're still asleep, so I updated the output: test.cpp:30:1: error: static assertion failed due to requirement 'inv(true) == inv(false)' static_assert(inv(true) == inv(false)); ^ ~~~

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

tbaeder added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16574 + if (BoolValue) { +Str.push_back('t'); +Str.push_back('r'); I really hope there's a better way to do this that I just don't know about :) CHANGES SINCE LAST AC

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

tbaeder updated this revision to Diff 449589. tbaeder added a comment. I just took the plunge and made `DiagnoseStaticAssertDetails()` take into account all static assertions, regardless of integer literals. It now also prints integers, booleans, chars and floats correctly. For example: test

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

aaron.ballman added a comment. Wow, I *really* like this improvement, thank you for working on it! This is going to be especially helpful for things like `static_assert(foo() == 12, "oh no, foo wasn't 12!");`. Can you also add a release note for this improvement? Comment at:

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

tbaeder added a comment. Oh, I noticed that g++ already does this: test.cpp:27:21: error: static assertion failed 27 | static_assert(foo() < 4); | ~~^~~ test.cpp:27:21: note: the comparison reduces to ‘(10 < 4)’ CHANGES SINCE LAST ACTION https://reviews.llvm

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

tbaeder updated this revision to Diff 449226. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaDeclCXX.cpp clang/test/CXX/dcl.decl/

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

tbaeder updated this revision to Diff 449199. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130894/new/ https://reviews.llvm.org/D130894 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaDeclCXX.cpp clang/test/CXX/dcl.decl/

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

tbaeder added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16656 && !isa(InnerCond)) { + Diag(StaticAssertLoc, diag::err_static_assert_requirement_failed) Oops, this needs to go. Repository: rG LLVM Github M

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

tbaeder created this revision. tbaeder added a reviewer: aaron.ballman. Herald added a project: All. tbaeder requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. For c++ static constexpr m = 10; static_assert(m == 11); the output is befo