[libc++] LWG-4021 "mdspan::is_always_meow() should be noexcept", use LIBCPP_STATIC_ASSERT for noexcept strengthening
#74254
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…g in MSVC's STL. MSVC's STL conditionally strengthens `mdspan` construction/`is_meow`/`stride` and `elements_view` iterator `base() &&`, and always strengthens `mdspan` `is_always_meow` and `basic_stringbuf` `swap`, as permitted by the Standard.
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Stephan T. Lavavej (StephanTLavavej) ChangesFound while running libc++'s test suite with MSVC's STL.
Full diff: https://github.com/llvm/llvm-project/pull/74254.diff 10 Files Affected:
diff --git a/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_array.pass.cpp b/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_array.pass.cpp
index 2e5c842b50d45..e0ce58c96f6bd 100644
--- a/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_array.pass.cpp
+++ b/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_array.pass.cpp
@@ -65,7 +65,7 @@ test_mdspan_ctor_array(const H& handle, const M& map, const A&, std::array<typen
}
}
- static_assert(!noexcept(MDS(handle, exts)));
+ LIBCPP_STATIC_ASSERT(!noexcept(MDS(handle, exts)));
static_assert(check_mdspan_ctor_implicit<MDS, decltype(exts)> == (N == MDS::rank_dynamic()));
diff --git a/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_extents.pass.cpp b/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_extents.pass.cpp
index 007ab9cdc636d..508ed7e3089ee 100644
--- a/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_extents.pass.cpp
+++ b/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_extents.pass.cpp
@@ -51,7 +51,7 @@ constexpr void test_mdspan_types(const H& handle, const M& map, const A&) {
assert((H::move_counter() == 1));
}
}
- static_assert(!noexcept(MDS(handle, map.extents())));
+ LIBCPP_STATIC_ASSERT(!noexcept(MDS(handle, map.extents())));
assert(m.extents() == map.extents());
if constexpr (std::equality_comparable<H>)
assert(m.data_handle() == handle);
diff --git a/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_map.pass.cpp b/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_map.pass.cpp
index 4c636255c1c78..3cb5bfe355145 100644
--- a/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_map.pass.cpp
+++ b/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_map.pass.cpp
@@ -48,7 +48,7 @@ constexpr void test_mdspan_types(const H& handle, const M& map, const A&) {
assert((H::move_counter() == 1));
}
}
- static_assert(!noexcept(MDS(handle, map)));
+ LIBCPP_STATIC_ASSERT(!noexcept(MDS(handle, map)));
assert(m.extents() == map.extents());
if constexpr (std::equality_comparable<H>)
assert(m.data_handle() == handle);
diff --git a/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_map_acc.pass.cpp b/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_map_acc.pass.cpp
index 76ca7810963df..a709b5d2d8169 100644
--- a/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_map_acc.pass.cpp
+++ b/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_map_acc.pass.cpp
@@ -43,7 +43,7 @@ constexpr void test_mdspan_types(const H& handle, const M& map, const A& acc) {
assert((H::move_counter() == 1));
}
}
- static_assert(!noexcept(MDS(handle, map, acc)));
+ LIBCPP_STATIC_ASSERT(!noexcept(MDS(handle, map, acc)));
assert(m.extents() == map.extents());
if constexpr (std::equality_comparable<H>)
assert(m.data_handle() == handle);
diff --git a/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_span.pass.cpp b/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_span.pass.cpp
index ad4da40630c97..2b712396cc26b 100644
--- a/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_span.pass.cpp
+++ b/libcxx/test/std/containers/views/mdspan/mdspan/ctor.dh_span.pass.cpp
@@ -65,7 +65,7 @@ test_mdspan_ctor_span(const H& handle, const M& map, const A&, std::span<typenam
}
}
- static_assert(!noexcept(MDS(handle, exts)));
+ LIBCPP_STATIC_ASSERT(!noexcept(MDS(handle, exts)));
static_assert(check_mdspan_ctor_implicit<MDS, decltype(exts)> == (N == MDS::rank_dynamic()));
diff --git a/libcxx/test/std/containers/views/mdspan/mdspan/properties.pass.cpp b/libcxx/test/std/containers/views/mdspan/mdspan/properties.pass.cpp
index 35534fa879548..20f56d1fe2642 100644
--- a/libcxx/test/std/containers/views/mdspan/mdspan/properties.pass.cpp
+++ b/libcxx/test/std/containers/views/mdspan/mdspan/properties.pass.cpp
@@ -141,12 +141,12 @@ constexpr void test_mdspan_types(const H& handle, const M& map, const A& acc) {
ASSERT_SAME_TYPE(decltype(m.is_unique()), bool);
ASSERT_SAME_TYPE(decltype(m.is_exhaustive()), bool);
ASSERT_SAME_TYPE(decltype(m.is_strided()), bool);
- assert(!noexcept(MDS::is_always_unique()));
- assert(!noexcept(MDS::is_always_exhaustive()));
- assert(!noexcept(MDS::is_always_strided()));
- assert(!noexcept(m.is_unique()));
- assert(!noexcept(m.is_exhaustive()));
- assert(!noexcept(m.is_strided()));
+ LIBCPP_STATIC_ASSERT(!noexcept(MDS::is_always_unique()));
+ LIBCPP_STATIC_ASSERT(!noexcept(MDS::is_always_exhaustive()));
+ LIBCPP_STATIC_ASSERT(!noexcept(MDS::is_always_strided()));
+ LIBCPP_STATIC_ASSERT(!noexcept(m.is_unique()));
+ LIBCPP_STATIC_ASSERT(!noexcept(m.is_exhaustive()));
+ LIBCPP_STATIC_ASSERT(!noexcept(m.is_strided()));
assert(MDS::is_always_unique() == M::is_always_unique());
assert(MDS::is_always_exhaustive() == M::is_always_exhaustive());
assert(MDS::is_always_strided() == M::is_always_strided());
@@ -159,7 +159,7 @@ constexpr void test_mdspan_types(const H& handle, const M& map, const A& acc) {
if (m.is_strided()) {
for (typename MDS::rank_type r = 0; r < MDS::rank(); r++) {
ASSERT_SAME_TYPE(decltype(m.stride(r)), typename MDS::index_type);
- assert(!noexcept(m.stride(r)));
+ LIBCPP_STATIC_ASSERT(!noexcept(m.stride(r)));
assert(m.stride(r) == map.stride(r));
}
}
diff --git a/libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/member_swap_noexcept.pass.cpp b/libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/member_swap_noexcept.pass.cpp
index cdb09df7c7a9a..8feb0a7ca8121 100644
--- a/libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/member_swap_noexcept.pass.cpp
+++ b/libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/member_swap_noexcept.pass.cpp
@@ -84,7 +84,7 @@ static void test() {
{
std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_not_empty<CharT>> buf1;
std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_not_empty<CharT>> buf;
- static_assert(!noexcept(buf.swap(buf1)));
+ LIBCPP_STATIC_ASSERT(!noexcept(buf.swap(buf1)));
}
{
std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_propagate_on_container_swap_not_empty<CharT>> buf1;
diff --git a/libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/nonmember_swap_noexcept.pass.cpp b/libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/nonmember_swap_noexcept.pass.cpp
index fdefc5ebe9af0..c8f94c4dbf897 100644
--- a/libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/nonmember_swap_noexcept.pass.cpp
+++ b/libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.assign/nonmember_swap_noexcept.pass.cpp
@@ -83,7 +83,7 @@ static void test() {
{
std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_not_empty<CharT>> buf1;
std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_not_empty<CharT>> buf;
- static_assert(!noexcept(swap(buf, buf1)));
+ LIBCPP_STATIC_ASSERT(!noexcept(swap(buf, buf1)));
}
{
std::basic_stringbuf<CharT, std::char_traits<CharT>, test_alloc_propagate_on_container_swap_not_empty<CharT>> buf1;
diff --git a/libcxx/test/std/iterators/iterator.requirements/iterator.cust/iterator.cust.move/iter_move.pass.cpp b/libcxx/test/std/iterators/iterator.requirements/iterator.cust/iterator.cust.move/iter_move.pass.cpp
index 566638263e887..9f293ff483cd8 100644
--- a/libcxx/test/std/iterators/iterator.requirements/iterator.cust/iterator.cust.move/iter_move.pass.cpp
+++ b/libcxx/test/std/iterators/iterator.requirements/iterator.cust/iterator.cust.move/iter_move.pass.cpp
@@ -158,15 +158,15 @@ constexpr bool test() {
auto unscoped = check_unqualified_lookup::unscoped_enum::a;
assert(std::ranges::iter_move(unscoped) == check_unqualified_lookup::unscoped_enum::a);
- assert(!noexcept(std::ranges::iter_move(unscoped)));
+ static_assert(!noexcept(std::ranges::iter_move(unscoped)));
auto scoped = check_unqualified_lookup::scoped_enum::a;
assert(std::ranges::iter_move(scoped) == nullptr);
- assert(noexcept(std::ranges::iter_move(scoped)));
+ static_assert(noexcept(std::ranges::iter_move(scoped)));
auto some_union = check_unqualified_lookup::some_union{0};
assert(std::ranges::iter_move(some_union) == 0);
- assert(!noexcept(std::ranges::iter_move(some_union)));
+ static_assert(!noexcept(std::ranges::iter_move(some_union)));
// Check noexcept-correctness
static_assert(noexcept(std::ranges::iter_move(std::declval<WithADL<true>>())));
diff --git a/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/base.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/base.pass.cpp
index 3729c8e543113..79c4e1f418372 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/base.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.elements/iterator/base.pass.cpp
@@ -33,7 +33,7 @@ using ElementsIter = std::ranges::iterator_t<std::ranges::elements_view<BaseView
static_assert(IsBaseNoexcept<const ElementsIter&>);
static_assert(IsBaseNoexcept<ElementsIter&>);
static_assert(IsBaseNoexcept<const ElementsIter&&>);
-static_assert(!IsBaseNoexcept<ElementsIter&&>);
+LIBCPP_STATIC_ASSERT(!IsBaseNoexcept<ElementsIter&&>);
constexpr bool test() {
std::tuple<int> t{5};
|
philnik777
requested changes
Dec 3, 2023
There was a problem hiding this comment.
I'd like to have an answer first, since I feel like I'm missing something here that explains the noexcept missing from the standard. If that is the case, we may have to fix the test differently, and the MSVC STL would probably also be wrong in making things noexcept unconditionally.
| assert(!noexcept(m.is_unique())); | ||
| assert(!noexcept(m.is_exhaustive())); | ||
| assert(!noexcept(m.is_strided())); | ||
| LIBCPP_STATIC_ASSERT(!noexcept(MDS::is_always_unique())); |
There was a problem hiding this comment.
@crtrott Why have we added these tests in the first place? And why doesn't the standard require them to be noexcept? From what I can tell, these should meet the Lakos rule requirements. Same questions for the other mdspan changes.
There was a problem hiding this comment.
@crtrott can answer definitively, but it appeared to me that these tests were semi-mechanically translating the Standardese, and it was a simple oversight that implementations don't have to follow the absence of noexcept in the Standard.
For mdspan::is_always_meow(), there's a simple proof that the Standard could absolutely mark them as unconditionally noexcept. N4964 [mdspan.mdspan.overview] requires mdspan::is_always_meow() to be effects-equivalent-to { return mapping_type::is_always_meow(); }, then the layout mapping requirements in [mdspan.layout.reqmts]/22, /24, /26 require mapping_type::is_always_meow() to be a constant expression. This could likely be resolved through a simple LWG issue.
For the other mdspan operations involved here (construction/is_meow/stride), we have conditionally strengthened them, and the Standard doesn't usually depict conditional noexcept unless it's really important, so that's likely the reason.
There was a problem hiding this comment.
I think I'd like us to file an LWG issue for this, update our implementation and make these static_asserts unconditionally with a note that we've implemented a not-yet-accepted LWG issue. For the non-unconditionally noexcept ones we can leave them as LIBCPP_STATIC_ASSERTs.
There was a problem hiding this comment.
I've submitted an LWG issue and as soon as I get a number I can update your product and test code here.
(The LWG issue covers only mdspan::is_always_meow(). Our basic_stringbuf swap strengthening is for something that technically has preconditions, since non-equal non-POCS allocators are forbidden at runtime.)
Also, we can portably upgrade `assert` to `static_assert` when checking `MDS::is_always_meow() == M::is_always_meow()`.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
StephanTLavavej
changed the title
LIBCPP_STATIC_ASSERT to deal with noexcept strengtheningmdspan::is_always_meow() should be noexcept", use LIBCPP_STATIC_ASSERT for noexcept strengthening
|
@philnik777 I've gotten an LWG issue number and updated the product/test code accordingly. I've also updated the PR title and description. |
philnik777
approved these changes
Dec 10, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Found while running libc++'s test suite with MSVC's STL.
mdspan::is_always_meow()should benoexcept" and implemented this in libc++'s product and test code.LIBCPP_STATIC_ASSERTto avoid issues withnoexceptstrengthening in MSVC's STL.mdspanconstruction/is_meow/strideandelements_viewiteratorbase() &&, and always strengthensbasic_stringbufswap.mdspan/properties.pass.cpp, this also upgrades runtimeasserts tostatic_asserts.asserttostatic_assertwhen inspecting thenoexceptness ofstd::ranges::iter_move. (These!noexcepttests weren't causing issues for MSVC's STL, so I didn't change them to be libc++-specific.)