[PATCH] D105127: Implement P1401R5

aaron.ballman closed this revision. aaron.ballman added a comment. Thanks for the patch! I've committed on your behalf in 8747234032c9c6270a6198ab3cca14ce2bd18721 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D105127: Implement P1401R5

cor3ntin updated this revision to Diff 357536. cor3ntin added a comment. Fix test formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105127/new/ https://reviews.llvm.org/D105127 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D105127: Implement P1401R5

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Oops, I chanted the magic incantation but forgot to press the button. LGTM again. ;-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D105127: Implement P1401R5

aaron.ballman added a comment. This LGTM aside from a small nit, but you should wait a bit to land in case other reviewers have comments. Comment at: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp:42-43 + +struct S { +} s; void f() { This looks like

[PATCH] D105127: Implement P1401R5

cor3ntin marked 5 inline comments as done. cor3ntin added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1491 +def err_constexpr_if_condition_expression_is_not_constant : Error< + "constexpr if condition is not a constant expression convertible to

[PATCH] D105127: Implement P1401R5

cor3ntin updated this revision to Diff 357501. cor3ntin marked 3 inline comments as done. cor3ntin added a comment. Improve diagnostic message, fix comments, use isInvalid instead of isUsable Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D105127: Implement P1401R5

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. Making it clear that there are still some minor changes left to be made. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D105127: Implement P1401R5

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1491 +def err_constexpr_if_condition_expression_is_not_constant : Error< + "constexpr if condition is not a constant expression convertible to bool">; def err_static_assert_failed

[PATCH] D105127: Implement P1401R5

cor3ntin updated this revision to Diff 357210. cor3ntin added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105127/new/ https://reviews.llvm.org/D105127 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D105127: Implement P1401R5

cor3ntin updated this revision to Diff 356022. cor3ntin added a comment. Missing EOL Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105127/new/ https://reviews.llvm.org/D105127 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D105127: Implement P1401R5

mizvekov added inline comments. Comment at: clang/test/SemaCXX/static-assert.cpp:202 +static_assert(constexprNotBool, "message"); // expected-error {{value of type 'const NotBool' is not contextually convertible to 'bool'}} \ No newline at end of file Missing

[PATCH] D105127: Implement P1401R5

cor3ntin updated this revision to Diff 355861. cor3ntin added a comment. Mark the feature completion as partial as this PR does not fix explicit(bool) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105127/new/ https://reviews.llvm.org/D105127

[PATCH] D105127: Implement P1401R5

rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1491 +def err_constexpr_if_condition_expression_is_not_constant : Error< + "constexpr if condition is not a constant expression convertible to bool">; def err_static_assert_failed :

[PATCH] D105127: Implement P1401R5

cor3ntin updated this revision to Diff 355720. cor3ntin added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105127/new/ https://reviews.llvm.org/D105127 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D105127: Implement P1401R5

cor3ntin updated this revision to Diff 355706. cor3ntin added a comment. - Apply the change to all C++ version - Add more tests for static_assert Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105127/new/ https://reviews.llvm.org/D105127 Files:

[PATCH] D105127: Implement P1401R5

cor3ntin added inline comments. Comment at: clang/www/cxx_status.html:1299 https://wg21.link/P1401R5;>P1401R5 - No + Clang 13 rsmith wrote: > This should be class `unreleased` (yellow) for now so that people can easily > tell what's in

[PATCH] D105127: Implement P1401R5

cor3ntin marked 2 inline comments as done. cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:3929 + + if (IsConstexpr && !LangOpts.CPlusPlus2b && !CondExpr->isValueDependent()) { +llvm::APSInt Value(/*BitWidth*/ 1); rsmith wrote: >

[PATCH] D105127: Implement P1401R5

cor3ntin updated this revision to Diff 355698. cor3ntin added a comment. Replace !isUsable() by isInvalid() Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105127/new/ https://reviews.llvm.org/D105127 Files:

[PATCH] D105127: Implement P1401R5

cor3ntin updated this revision to Diff 355697. cor3ntin added a comment. Mark the feature as unreleased Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105127/new/ https://reviews.llvm.org/D105127 Files:

[PATCH] D105127: Implement P1401R5

rsmith added inline comments. Comment at: clang/www/cxx_status.html:1299 https://wg21.link/P1401R5;>P1401R5 - No + Clang 13 This should be class `unreleased` (yellow) for now so that people can easily tell what's in the most recent Clang

[PATCH] D105127: Implement P1401R5

rsmith added a comment. In D105127#2851443 , @rsmith wrote: > P1401R5 points out that we get `noexcept(bool)` wrong in the opposite > direction, permitting conversions to `bool` that we are supposed to reject. > While that rule didn't change as part of

[PATCH] D105127: Implement P1401R5

rsmith added a comment. Would we benefit from any more test coverage for the `static_assert` side of this paper, or is our (formerly wrong and now correct) behavior already well-covered by existing tests? (Similarly for `explicit(bool)`, do we already have good test coverage for the

[PATCH] D105127: Implement P1401R5

rsmith added a comment. P1401R5 points out that we get `noexcept(bool)` wrong in the opposite direction, permitting conversions to `bool` that we are supposed to reject. While that rule didn't change as part of P1401R5, it'd be nice to handle it as part of dealing with that paper.

[PATCH] D105127: Implement P1401R5

mizvekov added a comment. LGTM! Since you submitted this quite recently, I would wait some more time before landing, for any other people who might want to review this. Maybe wait for it to be a week old, ping anyone else you might want to get a review from, and wait a couple of days for them

[PATCH] D105127: Implement P1401R5

cor3ntin updated this revision to Diff 355666. cor3ntin added a comment. Fix Formatting in tests. Do not return early for value dependent expressions as PerformContextuallyConvertToBool performs checks that are expected to occur down the line Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D105127: Implement P1401R5

cor3ntin added a comment. In D105127#2850975 , @mizvekov wrote: > So I read the paper, downloaded this patch, played around with it a little > bit, tried some different tests, like expressions with dependent types, > classes with regular/explicit

[PATCH] D105127: Implement P1401R5

mizvekov added a comment. Its unfortunate the buildbot does not have debug symbols. If you have not managed to reproduce it locally, here is the backtrace I got: FAIL: Clang :: CXX/except/except.spec/p1.cpp (1 of 1) TEST 'Clang :: CXX/except/except.spec/p1.cpp' FAILED

[PATCH] D105127: Implement P1401R5

cor3ntin added a comment. In D105127#2850975 , @mizvekov wrote: > So I read the paper, downloaded this patch, played around with it a little > bit, tried some different tests, like expressions with dependent types, > classes with regular/explicit

[PATCH] D105127: Implement P1401R5

mizvekov requested changes to this revision. mizvekov added a comment. This revision now requires changes to proceed. So I read the paper, downloaded this patch, played around with it a little bit, tried some different tests, like expressions with dependent types, classes with regular/explicit

[PATCH] D105127: Implement P1401R5

cor3ntin added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1491 +def err_constexpr_if_condition_expression_is_not_constant : Error< + "constexpr if condition is not a constant expression convertible to bool">; def err_static_assert_failed :

[PATCH] D105127: Implement P1401R5

cor3ntin updated this revision to Diff 355505. cor3ntin marked an inline comment as done. cor3ntin added a comment. Improve and fix test, Move the fixme in a more sensible place Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105127/new/

[PATCH] D105127: Implement P1401R5

mizvekov added a comment. Thanks! Just some nits and some minor points on the tests. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1491 +def err_constexpr_if_condition_expression_is_not_constant : Error< + "constexpr if condition is not a constant expression

[PATCH] D105127: Implement P1401R5

cor3ntin updated this revision to Diff 355477. cor3ntin added a comment. Fix formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105127/new/ https://reviews.llvm.org/D105127 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D105127: Implement P1401R5

cor3ntin updated this revision to Diff 355462. cor3ntin added a comment. Fix and add tests for the case where the condition is not convertible to bool Simplify code. Add reference to c++23 wording Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D105127: Implement P1401R5

mizvekov added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:3930 + ExprResult E = PerformContextuallyConvertToBool(CondExpr); + if (!IsConstexpr || CondExpr->isValueDependent()) +return E; Minor nit: I think it would be a tiny bit clearer if

[PATCH] D105127: Implement P1401R5

cor3ntin created this revision. cor3ntin requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Support Narrowing conversions to bool in if constexpr condition under C++23 language mode. Only if constexpr is implemented as the behavior of