From 8487e23e5bca6d4edda3a1d9d0237ef0a3fa8686 Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Sat, 4 Mar 2023 01:22:16 +0100 Subject: [PATCH 1/6] Implement *Mandates* clauses and even more cleanups! --- stl/inc/mdspan | 71 ++++++++++++++---------- tests/std/tests/P0009R18_mdspan/test.cpp | 2 + 2 files changed, 44 insertions(+), 29 deletions(-) diff --git a/stl/inc/mdspan b/stl/inc/mdspan index 7c057582d76..9ecff97fcf9 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -125,13 +125,11 @@ public: using size_type = make_unsigned_t; using rank_type = size_t; - // TRANSITION: doesn't account for extended integer types - static_assert(_Is_any_of_v, signed char, unsigned char, short, unsigned short, int, - unsigned int, long, unsigned long, long long, unsigned long long>, - "N4928 [mdspan.extents.overview]/2 " - "requires that extents::index_type be a signed or unsigned integer type."); - - static_assert(((_Extents == dynamic_extent || _Extents <= (numeric_limits<_IndexType>::max)()) && ...)); + 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); @@ -242,10 +240,10 @@ extents(_Integrals... _Ext) -> extents, _Integrals>::value...>; template -constexpr bool _Is_extents = false; +constexpr bool _Is_extents_v = false; template -constexpr bool _Is_extents> = true; +constexpr bool _Is_extents_v> = true; template struct _Layout_mapping_alike_helper : false_type {}; @@ -256,7 +254,7 @@ struct _Layout_mapping_alike_helper<_Mapping, is_same, is_same, bool_constant<_Mapping::is_always_strided()>, bool_constant<_Mapping::is_always_exhaustive()>, bool_constant<_Mapping::is_always_unique()>>> - : bool_constant<_Is_extents> {}; + : bool_constant<_Is_extents_v> {}; template struct _Layout_mapping_alike : bool_constant<_Layout_mapping_alike_helper<_Mapping>::value> {}; @@ -292,12 +290,12 @@ public: constexpr mapping() noexcept = default; constexpr mapping(const mapping&) noexcept = default; - constexpr mapping(const _Extents& e) noexcept : _Myext(e){}; + constexpr mapping(const _Extents& _Ext) noexcept : _Myext(_Ext) {} template , int> = 0> constexpr explicit(!is_convertible_v<_OtherExtents, _Extents>) mapping(const mapping<_OtherExtents>& _Other) noexcept - : _Myext{_Other.extents()} {}; + : _Myext{_Other.extents()} {} template , int> = 0> @@ -367,8 +365,8 @@ public: return _Result; } - template - _NODISCARD friend constexpr bool operator==(const mapping& _Left, const mapping& _Right) noexcept { + template + _NODISCARD_FRIEND constexpr bool operator==(const mapping& _Left, const mapping<_OtherExtents>& _Right) noexcept { return _Left.extents() == _Right.extents(); } @@ -397,12 +395,12 @@ public: constexpr mapping() noexcept = default; constexpr mapping(const mapping&) noexcept = default; - constexpr mapping(const _Extents& e) noexcept : _Myext(e){}; + constexpr mapping(const _Extents& _Ext) noexcept : _Myext(_Ext) {} template , int> = 0> constexpr explicit(!is_convertible_v<_OtherExtents, _Extents>) mapping(const mapping<_OtherExtents>& _Other) noexcept - : _Myext{_Other.extents()} {}; + : _Myext{_Other.extents()} {} template , int> = 0> @@ -472,8 +470,8 @@ public: return _Result; } - template - _NODISCARD friend constexpr bool operator==(const mapping& _Left, const mapping& _Right) noexcept { + template + _NODISCARD_FRIEND constexpr bool operator==(const mapping& _Left, const mapping<_OtherExtents>& _Right) noexcept { return _Left.extents() == _Right.extents(); } @@ -618,7 +616,7 @@ public: && extents_type::rank() == _OtherMapping::extents_type::rank() && _OtherMapping::is_always_strided(), int> = 0> - _NODISCARD friend constexpr bool operator==(const mapping& _Left, const _OtherMapping& _Right) noexcept { + _NODISCARD_FRIEND constexpr bool operator==(const mapping& _Left, const _OtherMapping& _Right) noexcept { if (_Left.extents() != _Right.extents()) { return false; } @@ -705,6 +703,17 @@ public: using data_handle_type = typename accessor_type::data_handle_type; using reference = typename accessor_type::reference; + static_assert( + sizeof(_ElementType) > 0, "ElementType must be a complete type (N4928 [mdspan.mdspan.overview]/2.1)."); + static_assert( + !is_abstract_v<_ElementType>, "ElementType cannot be an abstract type (N4928 [mdspan.mdspan.overview]/2.1)."); + static_assert( + !is_array_v<_ElementType>, "ElementType cannot be an array type (N4928 [mdspan.mdspan.overview]/2.1)."); + static_assert("Extents must be a specialization of extents (N4928 [mdspan.mdspan.overview]/2.2)."); + static_assert(is_same_v<_ElementType, typename _AccessorPolicy::element_type>, + "Expression is_same_v should be true (N4928 " + "[mdspan.mdspan.overview]/2.3)."); + _NODISCARD static constexpr rank_type rank() noexcept { return _Extents::rank(); } @@ -713,8 +722,8 @@ public: return _Extents::rank_dynamic(); } - _NODISCARD static constexpr size_t static_extent(const rank_type r) noexcept { - return _Extents::static_extent(r); + _NODISCARD static constexpr size_t static_extent(const rank_type _Rank) noexcept { + return _Extents::static_extent(_Rank); } template = 0> constexpr mdspan() {} - constexpr mdspan(const mdspan& rhs) = default; - constexpr mdspan(mdspan&& rhs) = default; + constexpr mdspan(const mdspan&) = default; + constexpr mdspan(mdspan&&) = default; template ) mdspan(const mdspan<_OtherElementType, _OtherExtents, _OtherLayoutPolicy, _OtherAccessor>& _Other) : _Ptr{_Other._Ptr}, _Map{_Other._Map}, _Acc{_Other._Acc} { - static_assert(is_constructible_v); - static_assert(is_constructible_v); + static_assert(is_constructible_v, + "Expression is_constructible_v should " + "be true (N4928 [mdspan.mdspan.cons]/20.1)."); + static_assert(is_constructible_v, + "Expression is_constructible_v should be true (N4928 " + "[mdspan.mdspan.cons]/20.2)."); } - constexpr mdspan& operator=(const mdspan& rhs) = default; - constexpr mdspan& operator=(mdspan&& rhs) = default; + constexpr mdspan& operator=(const mdspan&) = default; + constexpr mdspan& operator=(mdspan&&) = default; // TRANSITION operator[](const _OtherIndexTypes... _Indices) template struct stateful_accessor { + using element_type = Type; using data_handle_type = Type*; using reference = Type&; From 01cf15e167396b839b8771d0593593b617a9d4d4 Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Sat, 4 Mar 2023 03:21:15 +0100 Subject: [PATCH 2/6] Add `inline` to `_Is_extents_v` variable --- stl/inc/mdspan | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/mdspan b/stl/inc/mdspan index 9ecff97fcf9..e25357ec4da 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -240,10 +240,10 @@ extents(_Integrals... _Ext) -> extents, _Integrals>::value...>; template -constexpr bool _Is_extents_v = false; +inline constexpr bool _Is_extents_v = false; template -constexpr bool _Is_extents_v> = true; +inline constexpr bool _Is_extents_v> = true; template struct _Layout_mapping_alike_helper : false_type {}; From 3597d3c74c8de26028603d03145903768c249358 Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Sat, 4 Mar 2023 03:22:06 +0100 Subject: [PATCH 3/6] But remove `_v` suffix - see [mdspan.layout.stride.expo]/3 Since the standard suggests `is-extents` name, I'm not going to add extra suffix http://eel.is/c++draft/views.multidim#mdspan.layout.stride.expo-3 --- stl/inc/mdspan | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stl/inc/mdspan b/stl/inc/mdspan index e25357ec4da..89990ebbf01 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -240,10 +240,10 @@ extents(_Integrals... _Ext) -> extents, _Integrals>::value...>; template -inline constexpr bool _Is_extents_v = false; +inline constexpr bool _Is_extents = false; template -inline constexpr bool _Is_extents_v> = true; +inline constexpr bool _Is_extents> = true; template struct _Layout_mapping_alike_helper : false_type {}; @@ -254,7 +254,7 @@ struct _Layout_mapping_alike_helper<_Mapping, is_same, is_same, bool_constant<_Mapping::is_always_strided()>, bool_constant<_Mapping::is_always_exhaustive()>, bool_constant<_Mapping::is_always_unique()>>> - : bool_constant<_Is_extents_v> {}; + : bool_constant<_Is_extents> {}; template struct _Layout_mapping_alike : bool_constant<_Layout_mapping_alike_helper<_Mapping>::value> {}; From 954b88c993b8b1221731efc7481d859bd5d4e7c8 Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Sat, 4 Mar 2023 03:25:55 +0100 Subject: [PATCH 4/6] *`is-mapping-of`* missed `inline` too --- stl/inc/mdspan | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/mdspan b/stl/inc/mdspan index 89990ebbf01..8d427949c0f 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -260,7 +260,7 @@ template struct _Layout_mapping_alike : bool_constant<_Layout_mapping_alike_helper<_Mapping>::value> {}; template -constexpr bool _Is_mapping_of = +inline constexpr bool _Is_mapping_of = is_same_v, _Mapping>; struct layout_left { From 5dd13583a071e1a87002540e473aa13c6ba34eee Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Sat, 4 Mar 2023 07:59:07 +0100 Subject: [PATCH 5/6] The Show MUST Go On Co-authored-by: Matt Stephanson <68978048+MattStephanson@users.noreply.github.com> --- stl/inc/mdspan | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stl/inc/mdspan b/stl/inc/mdspan index 8d427949c0f..71a5864ec2c 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -711,7 +711,7 @@ public: !is_array_v<_ElementType>, "ElementType cannot be an array type (N4928 [mdspan.mdspan.overview]/2.1)."); static_assert("Extents must be a specialization of extents (N4928 [mdspan.mdspan.overview]/2.2)."); static_assert(is_same_v<_ElementType, typename _AccessorPolicy::element_type>, - "Expression is_same_v should be true (N4928 " + "Expression is_same_v must be true (N4928 " "[mdspan.mdspan.overview]/2.3)."); _NODISCARD static constexpr rank_type rank() noexcept { @@ -790,10 +790,10 @@ public: mdspan(const mdspan<_OtherElementType, _OtherExtents, _OtherLayoutPolicy, _OtherAccessor>& _Other) : _Ptr{_Other._Ptr}, _Map{_Other._Map}, _Acc{_Other._Acc} { static_assert(is_constructible_v, - "Expression is_constructible_v should " + "Expression is_constructible_v must " "be true (N4928 [mdspan.mdspan.cons]/20.1)."); static_assert(is_constructible_v, - "Expression is_constructible_v should be true (N4928 " + "Expression is_constructible_v must be true (N4928 " "[mdspan.mdspan.cons]/20.2)."); } From 307bbd1e1e75d3e8e987f4b4588f0be009b27917 Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Sat, 4 Mar 2023 09:03:12 +0100 Subject: [PATCH 6/6] More static assertions! --- stl/inc/mdspan | 29 +++++++++++++++++++++++- tests/std/tests/P0009R18_mdspan/test.cpp | 29 +++++++++++++----------- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/stl/inc/mdspan b/stl/inc/mdspan index 71a5864ec2c..83dbe8a9c1e 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -226,6 +226,14 @@ public: } } } + + _NODISCARD static constexpr bool _Is_index_space_size_representable() { + if constexpr (rank_dynamic() == 0 && rank() > 0) { + return _STD in_range((_Extents * ...)); + } else { + return true; + } + } }; template @@ -287,6 +295,12 @@ public: using rank_type = typename _Extents::rank_type; using layout_type = layout_left; + static_assert( + _Is_extents<_Extents>, "Extents must be a specialization of extents (N4928 [mdspan.layout.left.overview]/2)."); + static_assert(_Extents::_Is_index_space_size_representable(), + "If Extends::rank_dynamic() == 0 is true, then the size of the multidimensional index space Extents() is " + "representable as a value of type typename Extends::index_type."); + constexpr mapping() noexcept = default; constexpr mapping(const mapping&) noexcept = default; @@ -392,6 +406,12 @@ public: using rank_type = typename _Extents::rank_type; using layout_type = layout_right; + static_assert( + _Is_extents<_Extents>, "Extents must be a specialization of extents (N4928 [mdspan.layout.right.overview]/2)."); + static_assert(_Extents::_Is_index_space_size_representable(), + "If Extends::rank_dynamic() == 0 is true, then the size of the multidimensional index space Extents() is " + "representable as a value of type typename Extends::index_type."); + constexpr mapping() noexcept = default; constexpr mapping(const mapping&) noexcept = default; @@ -499,6 +519,12 @@ public: using rank_type = typename _Extents::rank_type; using layout_type = layout_stride; + static_assert(_Is_extents<_Extents>, + "Extents must be a specialization of extents (N4928 [mdspan.layout.stride.overview]/2)."); + static_assert(_Extents::_Is_index_space_size_representable(), + "If Extends::rank_dynamic() == 0 is true, then the size of the multidimensional index space Extents() is " + "representable as a value of type typename Extends::index_type."); + constexpr mapping() noexcept = default; constexpr mapping(const mapping&) noexcept = default; @@ -709,7 +735,8 @@ public: !is_abstract_v<_ElementType>, "ElementType cannot be an abstract type (N4928 [mdspan.mdspan.overview]/2.1)."); static_assert( !is_array_v<_ElementType>, "ElementType cannot be an array type (N4928 [mdspan.mdspan.overview]/2.1)."); - static_assert("Extents must be a specialization of extents (N4928 [mdspan.mdspan.overview]/2.2)."); + static_assert( + _Is_extents<_Extents>, "Extents must be a specialization of extents (N4928 [mdspan.mdspan.overview]/2.2)."); static_assert(is_same_v<_ElementType, typename _AccessorPolicy::element_type>, "Expression is_same_v must be true (N4928 " "[mdspan.mdspan.overview]/2.3)."); diff --git a/tests/std/tests/P0009R18_mdspan/test.cpp b/tests/std/tests/P0009R18_mdspan/test.cpp index 7592a5550d8..6dd899092af 100644 --- a/tests/std/tests/P0009R18_mdspan/test.cpp +++ b/tests/std/tests/P0009R18_mdspan/test.cpp @@ -31,14 +31,14 @@ struct Convertible { }; struct ConstructibleAndConvertible { - // convertible and noexcept constuctible + // convertible and noexcept constructible constexpr operator size_t() noexcept { return size_t{0}; }; }; struct ConstructibleAndConvertibleConst { - // convertible and noexcept constuctible + // convertible and noexcept constructible constexpr operator size_t() const noexcept { return size_t{0}; }; @@ -180,13 +180,13 @@ void extent_tests_copy_ctor_other() { static_assert(!is_constructible_v, extents>); static_assert(!is_constructible_v, extents>); - // Static extents are constuctible, but not convertible, from dynamic extents. + // Static extents are constructible, but not convertible, from dynamic extents. // static_assert(is_constructible_v, extents>); constexpr extents ex0{extents{}}; (void) ex0; static_assert(!is_convertible_v, extents>); - // Dynamic extents are constuctible and convertible from static extents. + // Dynamic extents are constructible and convertible from static extents. static_assert(is_constructible_v, extents>); extents{extents{}}; static_assert(is_convertible_v, extents>); @@ -776,8 +776,8 @@ void layout_stride_tests_ctor_other_extents() { constexpr extents e2{3}; constexpr array s{3, 1}; - constexpr layout_stride::mapping m1(e1, s); - constexpr layout_stride::mapping m2(m1); + constexpr layout_stride::mapping> m1(e1, s); + constexpr layout_stride::mapping> m2(m1); static_assert(m2.extents() == e1); static_assert(m2.strides() == s); @@ -928,10 +928,11 @@ void mdspan_tests_ctor_sizes() { static_assert((mds1.extents() == extents{})); static_assert(mds1.is_exhaustive()); - 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*, + // Pathological::Empty>); // Empty not convertible to size_type - // TRANSITION: this assert should be true + // 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 @@ -949,11 +950,13 @@ void mdspan_tests_ctor_array() { static_assert(mds1.data_handle() == arr); static_assert(mds1.extents() == extents{}); - 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>); // Empty not convertible to size_type - static_assert(!is_constructible_v, int*, - array>); // Pathological::Extents not constructible from int + // 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