From 4076f242aa7cff10cecea13b263353427d7eb38f Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Thu, 23 Mar 2023 16:51:52 +0100 Subject: [PATCH 01/10] Rework `std::extents` --- stl/inc/mdspan | 285 +++++++++--------- tests/std/test.lst | 1 + .../tests/P0009R18_mdspan_extents/test.cpp | 40 ++- .../P0009R18_mdspan_extents_death/env.lst | 4 + .../P0009R18_mdspan_extents_death/test.cpp | 92 ++++++ 5 files changed, 268 insertions(+), 154 deletions(-) create mode 100644 tests/std/tests/P0009R18_mdspan_extents_death/env.lst create mode 100644 tests/std/tests/P0009R18_mdspan_extents_death/test.cpp diff --git a/stl/inc/mdspan b/stl/inc/mdspan index adbe8e48913..006871fa901 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -24,135 +24,79 @@ _STL_DISABLE_CLANG_WARNINGS _STD_BEGIN -template -struct _Mdspan_extent_type { +_EXPORT_STD template +class extents { +public: using index_type = _IndexType; + using size_type = make_unsigned_t; + using rank_type = size_t; + + _NODISCARD static constexpr rank_type rank() noexcept { + return sizeof...(_Extents); + } + + static_assert(_Is_standard_integer, + "IndexType must be a signed or unsigned integer type (N4928 [mdspan.extents.overview]/1.1)."); + static_assert(((_Extents == dynamic_extent || _STD in_range(_Extents)) && ...), + "Each element of Extents must be either equal to dynamic_extent, or must be representable as a value of type " + "IndexType (N4928 [mdspan.extents.overview]/1.2)."); - _NODISCARD static constexpr auto _Get_dynamic_indices() noexcept { // TRANSITION consteval? - array _Result; +private: + _NODISCARD static _CONSTEVAL auto _Make_dynamic_indices() noexcept { +#pragma warning(push) // TRANSITION, "/analyze:only" BUG? +#pragma warning(disable : 28020) // The expression '0<=_Param_(1)&&_Param_(1)<=1-1' is not true at this call + array _Result{}; size_t _Counter = 0; - for (size_t _Ix = 0; _Ix < sizeof...(_Extents); ++_Ix) { - _Result[_Ix] = _Counter; - if (_Static_extents[_Ix] == dynamic_extent) { + for (size_t _Idx = 0; _Idx < rank(); ++_Idx) { + _Result[_Idx] = _Counter; + if (_Static_extents[_Idx] == dynamic_extent) { ++_Counter; } } + _Result[rank()] = _Counter; return _Result; +#pragma warning(pop) } - index_type _Dynamic_extents[_Rank_dynamic] = {}; - static constexpr size_t _Static_extents[sizeof...(_Extents)] = {_Extents...}; - static constexpr array _Dynamic_indices = _Get_dynamic_indices(); - - constexpr _Mdspan_extent_type() noexcept = default; - - template - requires (sizeof...(_OtherIndexTypes) == _Rank_dynamic) && (is_same_v<_OtherIndexTypes, index_type> && ...) - constexpr _Mdspan_extent_type(_OtherIndexTypes... _OtherExtents) noexcept : _Dynamic_extents{_OtherExtents...} {} - - template - requires (_Size == _Rank_dynamic) - constexpr _Mdspan_extent_type(span<_OtherIndexType, _Size> _Data, index_sequence<_Idx...>) noexcept - : _Dynamic_extents{static_cast(_STD as_const(_Data[_Idx]))...} {} - - template - requires (sizeof...(_OtherIndexTypes) == sizeof...(_Extents)) && (sizeof...(_Extents) != _Rank_dynamic) - && (is_same_v<_OtherIndexTypes, index_type> && ...) - constexpr _Mdspan_extent_type(_OtherIndexTypes... _OtherExtents) noexcept { - auto _It = _Dynamic_extents; - ((_Extents == dynamic_extent ? void(*_It++ = _OtherExtents) : void(_OtherExtents)), ...); - } - - template - requires (_Size == sizeof...(_Extents)) && (sizeof...(_Extents) != _Rank_dynamic) - constexpr _Mdspan_extent_type(span<_OtherIndexType, _Size> _Data, index_sequence<_Idx...>) noexcept - : _Dynamic_extents{{static_cast(_STD as_const(_Data[_Dynamic_indices[_Idx]]))...}} {} - - constexpr index_type* _Begin_dynamic_extents() noexcept { - return _Dynamic_extents; - } - - constexpr const index_type* _Begin_dynamic_extents() const noexcept { - return _Dynamic_extents; - } -}; - -template -struct _Mdspan_extent_type<_IndexType, 0, _Extents...> { - using index_type = _IndexType; - - static constexpr size_t _Static_extents[sizeof...(_Extents)] = {_Extents...}; + static constexpr array _Static_extents = {_Extents...}; + static constexpr array _Dynamic_indices = _Make_dynamic_indices(); - constexpr _Mdspan_extent_type() noexcept = default; - - template - requires (sizeof...(_IndexTypes) == sizeof...(_Extents)) - constexpr _Mdspan_extent_type(_IndexTypes... /*_OtherExtents*/) noexcept {} - - template - requires (_Size == sizeof...(_Extents)) || (_Size == 0) - constexpr _Mdspan_extent_type(span<_OtherIndexType, _Size>, index_sequence<_Idx...>) noexcept {} - - constexpr index_type* _Begin_dynamic_extents() noexcept { - return nullptr; + _NODISCARD static constexpr rank_type _Dynamic_index(rank_type _Idx) noexcept { + return _Dynamic_indices[_Idx]; } - constexpr const index_type* _Begin_dynamic_extents() const noexcept { - return nullptr; - } -}; - -template -struct _Mdspan_extent_type<_IndexType, 0> { - using index_type = _IndexType; - - constexpr index_type* _Begin_dynamic_extents() { - return nullptr; + _NODISCARD static constexpr rank_type _Dynamic_index_inv(rank_type _Idx) noexcept { + for (rank_type _Rdx = 0; _Rdx < rank(); ++_Rdx) { + if (_Dynamic_index(_Rdx + 1) == _Idx + 1) { + return _Rdx; + } + } + _STD unreachable(); } - constexpr const index_type* _Begin_dynamic_extents() const noexcept { - return nullptr; - } -}; + array _Dynamic_extents{}; -_EXPORT_STD -template -class extents : private _Mdspan_extent_type<_IndexType, ((_Extents == dynamic_extent) + ... + 0), _Extents...> { public: - using _Mybase = _Mdspan_extent_type<_IndexType, ((_Extents == dynamic_extent) + ... + 0), _Extents...>; - using index_type = typename _Mybase::index_type; - using size_type = make_unsigned_t; - using rank_type = size_t; - - static_assert(_Is_standard_integer<_IndexType>, - "IndexType must be a signed or unsigned integer type (N4928 [mdspan.extents.overview]/1.1)."); - static_assert(((_Extents == dynamic_extent || _STD in_range<_IndexType>(_Extents)) && ...), - "Each element of Extents must be either equal to dynamic_extent, or must be representable as a value of type " - "IndexType (N4928 [mdspan.extents.overview]/1.2)."); - - _NODISCARD static constexpr rank_type rank() noexcept { - return sizeof...(_Extents); - } - _NODISCARD static constexpr rank_type rank_dynamic() noexcept { - return ((_Extents == dynamic_extent) + ... + 0); + return _Dynamic_index(rank()); } _NODISCARD static constexpr size_t static_extent(const rank_type _Idx) noexcept { - return _Mybase::_Static_extents[_Idx]; + _STL_VERIFY(_Idx < rank(), "Index must be less than rank() (N4928 [mdspan.extents.obs]/1)"); + return _Static_extents[_Idx]; } _NODISCARD constexpr index_type extent(const rank_type _Idx) const noexcept { + _STL_VERIFY(_Idx < rank(), "Index must be less than rank() (N4928 [mdspan.extents.obs]/3)"); if constexpr (rank_dynamic() == 0) { - return static_cast(_Mybase::_Static_extents[_Idx]); + return static_cast(static_extent(_Idx)); } else if constexpr (rank_dynamic() == rank()) { - return _Mybase::_Dynamic_extents[_Idx]; + return _Dynamic_extents[_Idx]; } else { - const auto _Static_extent = _Mybase::_Static_extents[_Idx]; - if (_Static_extent == dynamic_extent) { - return _Mybase::_Dynamic_extents[_Mybase::_Dynamic_indices[_Idx]]; + if (static_extent(_Idx) == dynamic_extent) { + return _Dynamic_extents[_Dynamic_index(_Idx)]; } else { - return static_cast(_Static_extent); + return static_cast(static_extent(_Idx)); } } } @@ -160,15 +104,23 @@ public: constexpr extents() noexcept = default; template - requires (sizeof...(_OtherExtents) == sizeof...(_Extents)) + requires (sizeof...(_OtherExtents) == rank()) && ((_OtherExtents == dynamic_extent || _Extents == dynamic_extent || _OtherExtents == _Extents) && ...) constexpr explicit((((_Extents != dynamic_extent) && (_OtherExtents == dynamic_extent)) || ...) || (numeric_limits::max)() < (numeric_limits<_OtherIndexType>::max)()) extents(const extents<_OtherIndexType, _OtherExtents...>& _Other) noexcept { - auto _Dynamic_it = _Mybase::_Begin_dynamic_extents(); - for (rank_type _Dim = 0; _Dim < sizeof...(_Extents); ++_Dim) { - if (_Mybase::_Static_extents[_Dim] == dynamic_extent) { - *_Dynamic_it++ = _Other.extent(_Dim); + auto _It = _Dynamic_extents.begin(); + for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { + _STL_VERIFY(static_extent(_Idx) == dynamic_extent || static_extent(_Idx) == _Other.extent(_Idx), + "Value of other.extent(r) must be equal to extent(r) for each r for which extent(r) is a static extent " + "(N4928 [mdspan.extents.cons]/2.1)"); + _STL_VERIFY(_STD in_range(_Other.extent(_Idx)), + "Value of other.extent(r) must be representable as a value of type index_type for every rank index r " + "(N4928 [mdspan.extents.cons]/2.2)"); + + if (static_extent(_Idx) == dynamic_extent) { + *_It = static_cast(_Other.extent(_Idx)); + ++_It; } } } @@ -177,51 +129,93 @@ public: requires (is_convertible_v<_OtherIndexTypes, index_type> && ...) && (is_nothrow_constructible_v && ...) && (sizeof...(_OtherIndexTypes) == rank_dynamic() || sizeof...(_OtherIndexTypes) == rank()) - constexpr explicit extents(_OtherIndexTypes... _Exts) noexcept - : _Mybase{static_cast(_STD move(_Exts))...} {} + constexpr explicit extents(_OtherIndexTypes... _Exts) noexcept { + if constexpr ((_Is_standard_integer<_OtherIndexTypes> && ...)) { + _STL_VERIFY(sizeof...(_Exts) == 0 || ((_Exts >= 0 && _STD in_range(_Exts)) && ...), + "Either sizeof...(exts) must be equal to 0 or each element of exts must be nonnegative and must be " + "representable as value of type index_type (N4928 [mdspan.extents.cons]/7.2)"); + } + + if constexpr (sizeof...(_Exts) == rank_dynamic()) { + _Dynamic_extents = {static_cast(_STD move(_Exts))...}; + } else { + array _Exts_arr{static_cast(_STD move(_Exts))...}; + auto _It = _Dynamic_extents.begin(); + for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { + _STL_VERIFY( + static_extent(_Idx) == dynamic_extent || _STD cmp_equal(static_extent(_Idx), _Exts_arr[_Idx]), + "Value of exts_arr[r] must be equal to extent(r) for each r for which extent(r) is a static extent " + "(N4928 [mdspan.extents.cons]/7.1)"); + if (static_extent(_Idx) == dynamic_extent) { + *_It = _Exts_arr[_Idx]; + ++_It; + } + } + } + } + + template + requires is_convertible_v + && is_nothrow_constructible_v && (_Size != rank()) + constexpr explicit extents(span<_OtherIndexType, _Size> _Exts, index_sequence<_Indices...>) noexcept + : _Dynamic_extents{static_cast(_STD as_const(_Exts[_Indices]))...} { + if constexpr (_Is_standard_integer<_OtherIndexType> && _Size != 0) { + for (_OtherIndexType _Ext : _Exts) { + _STL_VERIFY(_Ext >= 0 && _STD in_range(_Ext), + "Either N must be zero or exts[r] must be nonnegative and must be representable as value of type " + "index_type for every rank index r (N4928 [mdspan.extents.cons]/10.2)"); + } + } + } + + template + requires is_convertible_v + && is_nothrow_constructible_v && (_Size == rank()) + constexpr explicit extents(span<_OtherIndexType, _Size> _Exts, index_sequence<_Indices...>) noexcept + : _Dynamic_extents{static_cast(_STD as_const(_Exts[_Dynamic_index_inv(_Indices)]))...} { + for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { + _STL_VERIFY(static_extent(_Idx) == dynamic_extent || static_extent(_Idx) == _Exts[_Idx], + "Value of exts[r] must be equal to extent(r) for each r for which extent(r) is a static extent (N4928 " + "[mdspan.extents.cons]/10.1)"); + + if constexpr (_Is_standard_integer<_OtherIndexType> && _Size != 0) { + _STL_VERIFY(_Exts[_Idx] >= 0 && _STD in_range(_Exts[_Idx]), + "Either N must be zero or exts[r] must be nonnegative and must be representable as value of type " + "index_type for every rank index r (N4928 [mdspan.extents.cons]/10.2)"); + } + } + } template requires is_convertible_v && is_nothrow_constructible_v && (_Size == rank_dynamic() || _Size == rank()) constexpr explicit(_Size != rank_dynamic()) extents(span<_OtherIndexType, _Size> _Exts) noexcept - : _Mybase{_Exts, make_index_sequence{}} {} + : extents(_Exts, make_index_sequence{}) {} template requires is_convertible_v && is_nothrow_constructible_v && (_Size == rank_dynamic() || _Size == rank()) constexpr explicit(_Size != rank_dynamic()) extents(const array<_OtherIndexType, _Size>& _Exts) noexcept - : _Mybase{span{_Exts}, make_index_sequence{}} {} + : extents(span{_Exts}, make_index_sequence{}) {} template _NODISCARD_FRIEND constexpr bool operator==( const extents& _Left, const extents<_OtherIndexType, _OtherExtents...>& _Right) noexcept { - if constexpr (sizeof...(_Extents) != sizeof...(_OtherExtents)) { + if constexpr (rank() != sizeof...(_OtherExtents)) { return false; - } - - for (size_t _Dim = 0; _Dim < sizeof...(_Extents); ++_Dim) { - if (_STD cmp_not_equal(_Left.extent(_Dim), _Right.extent(_Dim))) { - return false; - } - } - - return true; - } - - constexpr void _Fill_extents(index_type* _Out) const noexcept { - auto _Dynamic_it = _Mybase::_Begin_dynamic_extents(); - for (size_t _Dim = 0; _Dim < sizeof...(_Extents); ++_Dim) { - if (_Mybase::_Static_extents[_Dim] == dynamic_extent) { - *_Out++ = *_Dynamic_it++; - } else { - *_Out++ = static_cast(_Mybase::_Static_extents[_Dim]); + } else { + for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { + if (_STD cmp_not_equal(_Left.extent(_Idx), _Right.extent(_Idx))) { + return false; + } } + return true; } } - _NODISCARD static constexpr bool _Is_index_space_size_representable() { + _NODISCARD static _CONSTEVAL bool _Is_index_space_size_representable() { if constexpr (rank_dynamic() == 0 && rank() > 0) { return _STD in_range((_Extents * ...)); } else { @@ -230,20 +224,29 @@ public: } }; -_EXPORT_STD -template -using dextents = - decltype([](const _IndexType2, const index_sequence<_Seq...>) constexpr { - return extents<_IndexType2, ((void) _Seq, dynamic_extent)...>{}; - }(_IndexType{0}, make_index_sequence<_Rank>{})); - -// TRANSITION: why not `((void) _Ext, dynamic_extent)...`?! +#if defined(__clang__) || defined(__EDG__) // TRANSITION, REQUIRES REPORT (ICE) template requires (is_convertible_v<_Integrals, size_t> && ...) -extents(_Integrals... _Ext) +extents(_Integrals... _Exts) -> extents; +#else // ^^^ no workaround / workaround vvv +template + requires (is_convertible_v<_Integrals, size_t> && ...) +extents(_Integrals...) -> extents, _Integrals>::value...>; +#endif // ^^^ workaround ^^^ + +template +struct _Dextents_impl; + +template +struct _Dextents_impl<_IndexType, index_sequence<_Indices...>> { + using type = extents<_IndexType, ((void) _Indices, dynamic_extent)...>; +}; + +_EXPORT_STD template +using dextents = typename _Dextents_impl<_IndexType, make_index_sequence<_Rank>>::type; -template +template inline constexpr bool _Is_extents = false; template diff --git a/tests/std/test.lst b/tests/std/test.lst index b6b81ee1106..fb4b5f3f4d1 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -230,6 +230,7 @@ tests\LWG3610_iota_view_size_and_integer_class tests\P0009R18_mdspan tests\P0009R18_mdspan_default_accessor tests\P0009R18_mdspan_extents +tests\P0009R18_mdspan_extents_death tests\P0019R8_atomic_ref tests\P0024R2_parallel_algorithms_adjacent_difference tests\P0024R2_parallel_algorithms_adjacent_find diff --git a/tests/std/tests/P0009R18_mdspan_extents/test.cpp b/tests/std/tests/P0009R18_mdspan_extents/test.cpp index b73415b842d..aef8df66975 100644 --- a/tests/std/tests/P0009R18_mdspan_extents/test.cpp +++ b/tests/std/tests/P0009R18_mdspan_extents/test.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -60,7 +61,8 @@ constexpr void do_check_members(index_sequence) { static_assert(is_nothrow_default_constructible_v); // Check 'extent' observer - assert((((ext.extent(Indices) == Extents && Extents != dynamic_extent) || ext.extent(Indices) == 0) && ...)); + assert( + (((cmp_equal(ext.extent(Indices), Extents) && Extents != dynamic_extent) || ext.extent(Indices) == 0) && ...)); using OtherIndexType = conditional_t, long long, unsigned long long>; using Ext2 = extents; @@ -82,7 +84,7 @@ constexpr void do_check_members(index_sequence) { } { // Check construction from array and span - auto arr = to_array({ext.extent(Indices)...}); + array arr = {ext.extent(Indices)...}; Ext2 ext2a{arr}; assert(((ext.extent(Indices) == ext2a.extent(Indices)) && ...)); assert(ext == ext2a); @@ -160,15 +162,17 @@ constexpr void check_construction_from_extents_pack() { static_assert(!is_constructible_v>); } -#if 0 // FIXME Bug in array/span constructor? - { // Check postconditions [FIXME] - using Ext = extents; - array arr = {4, 4, 4}; - Ext ext{arr}; - Ext ext2{4, 4, 4}; + { // Check postconditions + using Ext = extents; + Ext ext{4, ConvertibleToInt{}, 4}; + Ext ext2{4, 1, 4}; assert(ext == ext2); } -#endif // Bug? + + { // Check construciton with integers with mismatched signs + using Ext = extents; + (void) Ext{4ull}; + } { // Check implicit conversions static_assert(NotImplicitlyConstructibleFrom, unsigned long long>); @@ -179,9 +183,9 @@ constexpr void check_construction_from_extents_pack() { constexpr void check_construction_from_array_and_span() { { // Check construction from arrays/spans with elements (not) convertible to index_type - using Ext = extents; + using Ext = extents; - array arr1 = {4, 5}; + array arr1 = {1, 5}; Ext ext1a{arr1}; span s1{arr1}; Ext ext1b{s1}; @@ -201,6 +205,16 @@ constexpr void check_construction_from_array_and_span() { static_assert(!is_constructible_v>); } + { // Check construciton with integers with mismatched signs + using Ext = extents; + + array arr = {4ull}; + (void) Ext{arr}; + + span s{arr}; + (void) Ext{s}; + } + { // Check construction from arrays/spans with elements that may throw during conversion to index_type using Ext = extents; static_assert(!is_constructible_v, 2>>); @@ -255,10 +269,10 @@ constexpr void check_equality_operator() { } constexpr bool test() { - // check_members(); // FIXME Definitely a bug. + check_members(); check_members(); check_members(); - // check_members(); // FIXME Bug in array/span constructor? + check_members(); check_members(); check_construction_from_other_extents(); check_construction_from_extents_pack(); diff --git a/tests/std/tests/P0009R18_mdspan_extents_death/env.lst b/tests/std/tests/P0009R18_mdspan_extents_death/env.lst new file mode 100644 index 00000000000..18e2d7c71ec --- /dev/null +++ b/tests/std/tests/P0009R18_mdspan_extents_death/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\concepts_latest_matrix.lst diff --git a/tests/std/tests/P0009R18_mdspan_extents_death/test.cpp b/tests/std/tests/P0009R18_mdspan_extents_death/test.cpp new file mode 100644 index 00000000000..2c80d249ab3 --- /dev/null +++ b/tests/std/tests/P0009R18_mdspan_extents_death/test.cpp @@ -0,0 +1,92 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include + +#include + +using namespace std; + +void test_static_extent_function_with_invalid_index() { + using E = extents; + // Index must be less than rank() + (void) E::static_extent(1); +} + +void test_extent_function_with_invalid_index() { + extents e; + // Index must be less than rank() + (void) e.extent(1); +} + +void test_construction_from_other_extents_with_invalid_values() { + extents e1{1, 2}; + // Value of other.extent(r) must be equal to extent(r) for each r for which extent(r) is a static extent + [[maybe_unused]] extents e2{e1}; +} + +void test_construction_from_other_extents_with_unrepresentable_as_index_type_values() { + extents e1{256}; + // Value of other.extent(r) must be representable as a value of type index_type for every rank index r + [[maybe_unused]] extents e2{e1}; +} + +void test_construction_from_pack_with_invalid_values() { + // Value of exts_arr[r] must be equal to extent(r) for each r for which extent(r) is a static extent + [[maybe_unused]] extents e{1, 1}; +} + +void test_construction_from_pack_with_unrepresentable_as_index_type_values() { + // Either sizeof...(exts) must be equal to 0 or each element of exts must be nonnegative and must be representable + // as value of type index_type + [[maybe_unused]] extents e{1, 256}; +} + +void test_construction_from_span_with_invalid_values() { + int vals[] = {1, 2}; + span s{vals}; + // Value of other.extent(r) must be equal to extent(r) for each r for which extent(r) is a static extent + [[maybe_unused]] extents e{s}; +} + +void test_construction_from_span_with_unrepresentable_as_index_type_values() { + int vals[] = {1, 2}; + span s{vals}; + // Value of other.extent(r) must be equal to extent(r) for each r for which extent(r) is a static extent + [[maybe_unused]] extents e{s}; +} + +void test_construction_from_array_with_invalid_values() { + int vals[] = {256}; + span s{vals}; + // Either N must be zero or exts[r] must be nonnegative and must be representable as value of type index_type for + // every rank index r + [[maybe_unused]] extents e{s}; +} + +void test_construction_from_array_with_unrepresentable_as_index_type_values() { + array a = {256}; + // Either N must be zero or exts[r] must be nonnegative and must be representable as value of type index_type for + // every rank index r + [[maybe_unused]] extents e{a}; +} + +int main(int argc, char* argv[]) { + std_testing::death_test_executive exec; + exec.add_death_tests({ + test_static_extent_function_with_invalid_index, + test_extent_function_with_invalid_index, + test_construction_from_other_extents_with_invalid_values, + test_construction_from_other_extents_with_unrepresentable_as_index_type_values, + test_construction_from_pack_with_invalid_values, + test_construction_from_pack_with_unrepresentable_as_index_type_values, + test_construction_from_span_with_invalid_values, + test_construction_from_span_with_unrepresentable_as_index_type_values, + test_construction_from_array_with_invalid_values, + test_construction_from_array_with_unrepresentable_as_index_type_values, + }); + return exec.run(argc, argv); +} From 13f34f4c76526066047d34e162e5276128ab3097 Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Thu, 23 Mar 2023 17:27:52 +0100 Subject: [PATCH 02/10] Fix older test suite --- stl/inc/mdspan | 5 ++++- tests/std/tests/P0009R18_mdspan/test.cpp | 21 ++++++++------------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/stl/inc/mdspan b/stl/inc/mdspan index 006871fa901..85f59260fc9 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -55,7 +55,7 @@ private: } _Result[rank()] = _Counter; return _Result; -#pragma warning(pop) +#pragma warning(pop) // TRANSITION, "/analyze:only" BUG? } static constexpr array _Static_extents = {_Extents...}; @@ -130,6 +130,8 @@ public: && (is_nothrow_constructible_v && ...) && (sizeof...(_OtherIndexTypes) == rank_dynamic() || sizeof...(_OtherIndexTypes) == rank()) constexpr explicit extents(_OtherIndexTypes... _Exts) noexcept { +#pragma warning(push) // TRANSITION, "/analyze:only" BUG? +#pragma warning(disable : 28020) // The expression '0<=_Param_(1)&&_Param_(1)<=1-1' is not true at this call if constexpr ((_Is_standard_integer<_OtherIndexTypes> && ...)) { _STL_VERIFY(sizeof...(_Exts) == 0 || ((_Exts >= 0 && _STD in_range(_Exts)) && ...), "Either sizeof...(exts) must be equal to 0 or each element of exts must be nonnegative and must be " @@ -152,6 +154,7 @@ public: } } } +#pragma warning(pop) // TRANSITION, "/analyze:only" BUG? } template diff --git a/tests/std/tests/P0009R18_mdspan/test.cpp b/tests/std/tests/P0009R18_mdspan/test.cpp index 6e0ad595689..7e4e53701c3 100644 --- a/tests/std/tests/P0009R18_mdspan/test.cpp +++ b/tests/std/tests/P0009R18_mdspan/test.cpp @@ -32,14 +32,14 @@ struct Convertible { struct ConstructibleAndConvertible { // convertible and noexcept constructible constexpr operator size_t() noexcept { - return size_t{0}; + return size_t{2}; }; }; struct ConstructibleAndConvertibleConst { // convertible and noexcept constructible constexpr operator size_t() const noexcept { - return size_t{0}; + return size_t{2}; }; }; @@ -139,20 +139,15 @@ void extent_tests_ctor_other_sizes() { static_assert(!is_constructible_v, Constructible>); static_assert(!is_constructible_v, Convertible>); // static_assert(is_constructible_v, ConstructibleAndConvertible>); - constexpr extents ex0{ConstructibleAndConvertible{}}; + [[maybe_unused]] constexpr extents ex0{ConstructibleAndConvertible{}}; // static_assert(is_constructible_v, ConstructibleAndConvertibleConst>); - constexpr extents ex1{ConstructibleAndConvertibleConst{}}; + [[maybe_unused]] constexpr extents ex1{ConstructibleAndConvertibleConst{}}; // static_assert(is_constructible_v, int>); - constexpr extents ex2(1); + [[maybe_unused]] constexpr extents ex2(1); static_assert(!is_constructible_v, int, int>); // static_assert(is_constructible_v, int, int, int>); - extents ex3(1, 2, 3); - - (void) ex0; - (void) ex1; - (void) ex2; - (void) ex3; + [[maybe_unused]] extents ex3(1, 2, 3); extents e0; assert(e0.extent(0) == 2); @@ -179,7 +174,7 @@ void extent_tests_copy_ctor_other() { // Static extents are constructible, but not convertible, from dynamic extents. // static_assert(is_constructible_v, extents>); - constexpr extents ex0{extents{}}; + constexpr extents ex0{extents{3}}; (void) ex0; static_assert(!is_convertible_v, extents>); @@ -239,7 +234,7 @@ void extent_tests_ctor_array() { static_assert(is_constructible_v, array>); constexpr extents ex3{array{}}; static_assert(is_constructible_v, array>); - constexpr extents ex4{array{}}; + constexpr extents ex4{array{10}}; static_assert(!is_constructible_v, array>); (void) ex3; (void) ex4; From 78dee94ff68d4118f903f556c81d99c65ed11d83 Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Thu, 23 Mar 2023 17:33:31 +0100 Subject: [PATCH 03/10] Fix more signed/unsigned comparisons --- stl/inc/mdspan | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/stl/inc/mdspan b/stl/inc/mdspan index 85f59260fc9..8a5d85f37cd 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -111,7 +111,8 @@ public: extents(const extents<_OtherIndexType, _OtherExtents...>& _Other) noexcept { auto _It = _Dynamic_extents.begin(); for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { - _STL_VERIFY(static_extent(_Idx) == dynamic_extent || static_extent(_Idx) == _Other.extent(_Idx), + _STL_VERIFY( + static_extent(_Idx) == dynamic_extent || _STD cmp_equal(static_extent(_Idx), _Other.extent(_Idx)), "Value of other.extent(r) must be equal to extent(r) for each r for which extent(r) is a static extent " "(N4928 [mdspan.extents.cons]/2.1)"); _STL_VERIFY(_STD in_range(_Other.extent(_Idx)), @@ -176,12 +177,11 @@ public: && is_nothrow_constructible_v && (_Size == rank()) constexpr explicit extents(span<_OtherIndexType, _Size> _Exts, index_sequence<_Indices...>) noexcept : _Dynamic_extents{static_cast(_STD as_const(_Exts[_Dynamic_index_inv(_Indices)]))...} { - for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { - _STL_VERIFY(static_extent(_Idx) == dynamic_extent || static_extent(_Idx) == _Exts[_Idx], - "Value of exts[r] must be equal to extent(r) for each r for which extent(r) is a static extent (N4928 " - "[mdspan.extents.cons]/10.1)"); - - if constexpr (_Is_standard_integer<_OtherIndexType> && _Size != 0) { + if constexpr (_Is_standard_integer<_OtherIndexType>) { + for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { + _STL_VERIFY(static_extent(_Idx) == dynamic_extent || _STD cmp_equal(static_extent(_Idx), _Exts[_Idx]), + "Value of exts[r] must be equal to extent(r) for each r for which extent(r) is a static extent " + "(N4928 [mdspan.extents.cons]/10.1)"); _STL_VERIFY(_Exts[_Idx] >= 0 && _STD in_range(_Exts[_Idx]), "Either N must be zero or exts[r] must be nonnegative and must be representable as value of type " "index_type for every rank index r (N4928 [mdspan.extents.cons]/10.2)"); From a50d4fe685da54abf2c2bcc409cd42a23e7bd17a Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Fri, 24 Mar 2023 11:17:38 +0100 Subject: [PATCH 04/10] Don't use `array` Addresses https://github.com/microsoft/STL/pull/3586#issuecomment-1482198182 --- stl/inc/mdspan | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/stl/inc/mdspan b/stl/inc/mdspan index 8a5d85f37cd..2f251f7440a 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -74,11 +74,20 @@ private: _STD unreachable(); } - array _Dynamic_extents{}; + struct _Static_extents_only { + constexpr _Static_extents_only(auto&&...) noexcept {} + + _NODISCARD constexpr index_type* begin() const noexcept { + return nullptr; + } + }; + + static constexpr rank_type _Rank_dynamic = _Dynamic_index(rank()); + conditional_t<_Rank_dynamic != 0, array, _Static_extents_only> _Dynamic_extents{}; public: _NODISCARD static constexpr rank_type rank_dynamic() noexcept { - return _Dynamic_index(rank()); + return _Rank_dynamic; } _NODISCARD static constexpr size_t static_extent(const rank_type _Idx) noexcept { From 99afffcad8d14d6cf87bc610cc1db8d1e7aca10e Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Fri, 24 Mar 2023 11:20:55 +0100 Subject: [PATCH 05/10] Add extra test in `mdspan_extents` --- tests/std/tests/P0009R18_mdspan_extents/test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/std/tests/P0009R18_mdspan_extents/test.cpp b/tests/std/tests/P0009R18_mdspan_extents/test.cpp index aef8df66975..dcada0e17e8 100644 --- a/tests/std/tests/P0009R18_mdspan_extents/test.cpp +++ b/tests/std/tests/P0009R18_mdspan_extents/test.cpp @@ -271,6 +271,7 @@ constexpr void check_equality_operator() { constexpr bool test() { check_members(); check_members(); + check_members(); check_members(); check_members(); check_members(); From b4130e01c8d438c062cda30dfeb881d7b3a028a3 Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Fri, 24 Mar 2023 11:27:12 +0100 Subject: [PATCH 06/10] Remove invalid tests and enable some from old suite --- tests/std/tests/P0009R18_mdspan/test.cpp | 26 +++++------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/tests/std/tests/P0009R18_mdspan/test.cpp b/tests/std/tests/P0009R18_mdspan/test.cpp index 7e4e53701c3..91ed592d099 100644 --- a/tests/std/tests/P0009R18_mdspan/test.cpp +++ b/tests/std/tests/P0009R18_mdspan/test.cpp @@ -138,15 +138,15 @@ void extent_tests_extent() { void extent_tests_ctor_other_sizes() { static_assert(!is_constructible_v, Constructible>); static_assert(!is_constructible_v, Convertible>); - // static_assert(is_constructible_v, ConstructibleAndConvertible>); + static_assert(is_constructible_v, ConstructibleAndConvertible>); [[maybe_unused]] constexpr extents ex0{ConstructibleAndConvertible{}}; - // static_assert(is_constructible_v, ConstructibleAndConvertibleConst>); + static_assert(is_constructible_v, ConstructibleAndConvertibleConst>); [[maybe_unused]] constexpr extents ex1{ConstructibleAndConvertibleConst{}}; - // static_assert(is_constructible_v, int>); + static_assert(is_constructible_v, int>); [[maybe_unused]] constexpr extents ex2(1); static_assert(!is_constructible_v, int, int>); - // static_assert(is_constructible_v, int, int, int>); + static_assert(is_constructible_v, int, int, int>); [[maybe_unused]] extents ex3(1, 2, 3); extents e0; @@ -173,7 +173,7 @@ void extent_tests_copy_ctor_other() { static_assert(!is_constructible_v, extents>); // Static extents are constructible, but not convertible, from dynamic extents. - // static_assert(is_constructible_v, extents>); + static_assert(is_constructible_v, extents>); constexpr extents ex0{extents{3}}; (void) ex0; static_assert(!is_convertible_v, extents>); @@ -914,14 +914,6 @@ void mdspan_tests_ctor_sizes() { static_assert((mds1.extents() == extents{})); static_assert(mds1.is_exhaustive()); - // TRANSITION: fix with concepts -> this is hard error per [mdspan.mdspan.overview]/2.2 - // static_assert(!is_constructible_v, int*, - // Pathological::Empty>); // Empty not convertible to size_type - - // TRANSITION: fix with concepts -> this is hard error per [mdspan.mdspan.overview]/2.2 - // static_assert(!is_constructible_v, int*, - // int>); // Pathological::Extents not constructible from int - static_assert(!is_constructible_v, Pathological::Layout>, int*, int>); // Pathological::Layout not constructible from extents @@ -936,14 +928,6 @@ void mdspan_tests_ctor_array() { static_assert(mds1.data_handle() == arr); static_assert(mds1.extents() == extents{}); - // TRANSITION: fix with concepts -> this is hard error per [mdspan.mdspan.overview]/2.2 - // static_assert(!is_constructible_v, int*, - // array>); // Empty not convertible to size_type - - // TRANSITION: fix with concepts -> this is hard error per [mdspan.mdspan.overview]/2.2 - // static_assert(!is_constructible_v, int*, - // array>); // Pathological::Extents not constructible from int - static_assert(!is_constructible_v, Pathological::Layout>, int*, array>); // Pathological::Layout not constructible from extents From e012636fc7a238a2fb4574bfb28e652083515ebf Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Fri, 24 Mar 2023 11:52:35 +0100 Subject: [PATCH 07/10] Cache `dynamic-index-inv` too --- stl/inc/mdspan | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/stl/inc/mdspan b/stl/inc/mdspan index 2f251f7440a..951664894c7 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -46,8 +46,8 @@ private: #pragma warning(push) // TRANSITION, "/analyze:only" BUG? #pragma warning(disable : 28020) // The expression '0<=_Param_(1)&&_Param_(1)<=1-1' is not true at this call array _Result{}; - size_t _Counter = 0; - for (size_t _Idx = 0; _Idx < rank(); ++_Idx) { + rank_type _Counter = 0; + for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { _Result[_Idx] = _Counter; if (_Static_extents[_Idx] == dynamic_extent) { ++_Counter; @@ -65,17 +65,27 @@ private: return _Dynamic_indices[_Idx]; } - _NODISCARD static constexpr rank_type _Dynamic_index_inv(rank_type _Idx) noexcept { - for (rank_type _Rdx = 0; _Rdx < rank(); ++_Rdx) { - if (_Dynamic_index(_Rdx + 1) == _Idx + 1) { - return _Rdx; + _NODISCARD static _CONSTEVAL auto _Make_dynamic_indices_inv() noexcept { + array _Result{}; + for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { + for (rank_type _Rdx = 0; _Rdx < rank(); ++_Rdx) { + if (_Dynamic_index(_Rdx + 1) == _Idx + 1) { + _Result[_Idx] = _Rdx; + break; + } } } - _STD unreachable(); + return _Result; + } + + static constexpr array _Dynamic_indices_inv = _Make_dynamic_indices_inv(); + + _NODISCARD static constexpr rank_type _Dynamic_index_inv(rank_type _Idx) noexcept { + return _Dynamic_indices_inv[_Idx]; } struct _Static_extents_only { - constexpr _Static_extents_only(auto&&...) noexcept {} + constexpr explicit _Static_extents_only(auto&&...) noexcept {} _NODISCARD constexpr index_type* begin() const noexcept { return nullptr; From 4f7a4bc31471053696e1033b1f8ea29c0cf4dfb2 Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Fri, 24 Mar 2023 11:59:42 +0100 Subject: [PATCH 08/10] Improve `_Static_extents_only` --- stl/inc/mdspan | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/stl/inc/mdspan b/stl/inc/mdspan index 951664894c7..28c902a855c 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -85,7 +85,10 @@ private: } struct _Static_extents_only { - constexpr explicit _Static_extents_only(auto&&...) noexcept {} + constexpr explicit _Static_extents_only() noexcept = default; + + template + constexpr explicit _Static_extents_only(_Args&&...) noexcept {} _NODISCARD constexpr index_type* begin() const noexcept { return nullptr; From 692fb6b477044eb925be61d452c221ee6dbd7463 Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Fri, 24 Mar 2023 13:07:56 +0100 Subject: [PATCH 09/10] Fix ICE on invalid (requires report) --- stl/inc/mdspan | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/mdspan b/stl/inc/mdspan index 28c902a855c..236b217afa1 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -87,7 +87,7 @@ private: struct _Static_extents_only { constexpr explicit _Static_extents_only() noexcept = default; - template + template constexpr explicit _Static_extents_only(_Args&&...) noexcept {} _NODISCARD constexpr index_type* begin() const noexcept { From 84a2accd9f397b0bdc985389bfb19a8883090f68 Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Fri, 24 Mar 2023 14:17:44 +0100 Subject: [PATCH 10/10] Improve test coverage --- .../tests/P0009R18_mdspan_extents/test.cpp | 50 +++++++++++++++++-- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/tests/std/tests/P0009R18_mdspan_extents/test.cpp b/tests/std/tests/P0009R18_mdspan_extents/test.cpp index dcada0e17e8..380aad6340c 100644 --- a/tests/std/tests/P0009R18_mdspan_extents/test.cpp +++ b/tests/std/tests/P0009R18_mdspan_extents/test.cpp @@ -164,9 +164,13 @@ constexpr void check_construction_from_extents_pack() { { // Check postconditions using Ext = extents; - Ext ext{4, ConvertibleToInt{}, 4}; - Ext ext2{4, 1, 4}; - assert(ext == ext2); + Ext ext1a{4, ConvertibleToInt{}, 4}; + Ext ext1b{4, 1, 4}; + assert(ext1a == ext1b); + + Ext ext2a{4, ConvertibleToInt{}}; + Ext ext2b{4, 1}; + assert(ext2a == ext2b); } { // Check construciton with integers with mismatched signs @@ -174,6 +178,11 @@ constexpr void check_construction_from_extents_pack() { (void) Ext{4ull}; } + { // Check narrowing conversions + using Ext = extents; + (void) Ext{4ll}; + } + { // Check implicit conversions static_assert(NotImplicitlyConstructibleFrom, unsigned long long>); static_assert(NotImplicitlyConstructibleFrom, long, long>); @@ -182,7 +191,7 @@ constexpr void check_construction_from_extents_pack() { } constexpr void check_construction_from_array_and_span() { - { // Check construction from arrays/spans with elements (not) convertible to index_type + { // Check construction from arrays/spans where [array/span].size() is equal to rank() using Ext = extents; array arr1 = {1, 5}; @@ -205,6 +214,29 @@ constexpr void check_construction_from_array_and_span() { static_assert(!is_constructible_v>); } + { // // Check construction from arrays/spans where [array/span].size() is equal to rank_dynamic() + using Ext = extents; + + array arr1 = {4, 4}; + Ext ext1a{arr1}; + span s1{arr1}; + Ext ext1b{s1}; + assert(ext1a == ext1b); + static_assert(is_nothrow_constructible_v); + static_assert(is_nothrow_constructible_v); + + array, 2> arr2; + Ext ext2a{arr2}; + span s2{arr2}; + Ext ext2b{s2}; + assert(ext2a == ext2b); + static_assert(is_nothrow_constructible_v); + static_assert(is_nothrow_constructible_v); + + static_assert(!is_constructible_v>); + static_assert(!is_constructible_v>); + } + { // Check construciton with integers with mismatched signs using Ext = extents; @@ -215,6 +247,16 @@ constexpr void check_construction_from_array_and_span() { (void) Ext{s}; } + { // Check narrowing conversions + using Ext = extents; + + array arr = {4ll}; + (void) Ext{arr}; + + span s{arr}; + (void) Ext{s}; + } + { // Check construction from arrays/spans with elements that may throw during conversion to index_type using Ext = extents; static_assert(!is_constructible_v, 2>>);