From f4cc153b6e9809a0f3f4071bd1469437c2b2a38c Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Wed, 26 Apr 2023 16:09:27 +0200 Subject: [PATCH 1/3] Improve debug experience a little bit --- stl/inc/mdspan | 99 +++++++++++++++++++++++--------------------------- 1 file changed, 46 insertions(+), 53 deletions(-) diff --git a/stl/inc/mdspan b/stl/inc/mdspan index 2fb443e92ec..b4e0a83afcb 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -32,45 +32,38 @@ public: 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 (N4944 [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 (N4944 [mdspan.extents.overview]/1.2)."); + static constexpr rank_type _Rank = sizeof...(_Extents); + static constexpr rank_type _Rank_dynamic = (static_cast(_Extents == dynamic_extent) + ... + 0); + private: + static constexpr array _Static_extents = {_Extents...}; + _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{}; + array _Result{}; rank_type _Counter = 0; - for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { + for (rank_type _Idx = 0; _Idx < _Rank; ++_Idx) { _Result[_Idx] = _Counter; if (_Static_extents[_Idx] == dynamic_extent) { ++_Counter; } } - _Result[rank()] = _Counter; + _Result[_Rank] = _Counter; return _Result; -#pragma warning(pop) // TRANSITION, "/analyze:only" BUG? } - static constexpr array _Static_extents = {_Extents...}; - static constexpr array _Dynamic_indices = _Make_dynamic_indices(); - - _NODISCARD static constexpr rank_type _Dynamic_index(rank_type _Idx) noexcept { - return _Dynamic_indices[_Idx]; - } + static constexpr array _Dynamic_indices = _Make_dynamic_indices(); _NODISCARD static _CONSTEVAL auto _Make_dynamic_indices_inv() noexcept { - array _Result{}; - for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { - for (rank_type _Idx_inv = 0; _Idx_inv < rank(); ++_Idx_inv) { - if (_Dynamic_index(_Idx_inv + 1) == _Idx + 1) { + array _Result{}; + for (rank_type _Idx = 0; _Idx < _Rank; ++_Idx) { + for (rank_type _Idx_inv = 0; _Idx_inv < _Rank; ++_Idx_inv) { + if (_Dynamic_indices[_Idx_inv + 1] == _Idx + 1) { _Result[_Idx] = _Idx_inv; break; } @@ -79,11 +72,7 @@ private: 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]; - } + static constexpr array _Dynamic_indices_inv = _Make_dynamic_indices_inv(); struct _Static_extents_only { constexpr explicit _Static_extents_only() noexcept = default; @@ -96,30 +85,33 @@ private: } }; - 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() noexcept { + return _Rank; + } + _NODISCARD static constexpr rank_type rank_dynamic() noexcept { return _Rank_dynamic; } _NODISCARD static constexpr size_t static_extent(const rank_type _Idx) noexcept { - _STL_VERIFY(_Idx < rank(), "Index must be less than rank() (N4944 [mdspan.extents.obs]/1)"); + _STL_VERIFY(_Idx < _Rank, "Index must be less than rank() (N4944 [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() (N4944 [mdspan.extents.obs]/3)"); + _STL_VERIFY(_Idx < _Rank, "Index must be less than rank() (N4944 [mdspan.extents.obs]/3)"); if constexpr (rank_dynamic() == 0) { - return static_cast(static_extent(_Idx)); + return static_cast(_Static_extents[_Idx]); } else if constexpr (rank_dynamic() == rank()) { return _Dynamic_extents[_Idx]; } else { - if (static_extent(_Idx) == dynamic_extent) { - return _Dynamic_extents[_Dynamic_index(_Idx)]; + if (_Static_extents[_Idx] == dynamic_extent) { + return _Dynamic_extents[_Dynamic_indices[_Idx]]; } else { - return static_cast(static_extent(_Idx)); + return static_cast(_Static_extents[_Idx]); } } } @@ -133,16 +125,16 @@ public: || (numeric_limits::max)() < (numeric_limits<_OtherIndexType>::max)()) extents(const extents<_OtherIndexType, _OtherExtents...>& _Other) noexcept { auto _It = _Dynamic_extents.begin(); - for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { + for (rank_type _Idx = 0; _Idx < _Rank; ++_Idx) { _STL_VERIFY( - static_extent(_Idx) == dynamic_extent || _STD cmp_equal(static_extent(_Idx), _Other.extent(_Idx)), + _Static_extents[_Idx] == dynamic_extent || _STD cmp_equal(_Static_extents[_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 " "(N4944 [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 " "(N4944 [mdspan.extents.cons]/2.2)"); - if (static_extent(_Idx) == dynamic_extent) { + if (_Static_extents[_Idx] == dynamic_extent) { *_It = static_cast(_Other.extent(_Idx)); ++_It; } @@ -167,12 +159,12 @@ public: } else { array _Exts_arr{static_cast(_STD move(_Exts))...}; auto _It = _Dynamic_extents.begin(); - for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { + for (rank_type _Idx = 0; _Idx < _Rank; ++_Idx) { _STL_VERIFY( - static_extent(_Idx) == dynamic_extent || _STD cmp_equal(static_extent(_Idx), _Exts_arr[_Idx]), + _Static_extents[_Idx] == dynamic_extent || _STD cmp_equal(_Static_extents[_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 " "(N4944 [mdspan.extents.cons]/7.1)"); - if (static_extent(_Idx) == dynamic_extent) { + if (_Static_extents[_Idx] == dynamic_extent) { *_It = _Exts_arr[_Idx]; ++_It; } @@ -199,10 +191,11 @@ public: 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)]))...} { + : _Dynamic_extents{static_cast(_STD as_const(_Exts[_Dynamic_indices_inv[_Indices]]))...} { 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]), + for (rank_type _Idx = 0; _Idx < _Rank; ++_Idx) { + _STL_VERIFY( + _Static_extents[_Idx] == dynamic_extent || _STD cmp_equal(_Static_extents[_Idx], _Exts[_Idx]), "Value of exts[r] must be equal to extent(r) for each r for which extent(r) is a static extent " "(N4944 [mdspan.extents.cons]/10.1)"); _STL_VERIFY(_Exts[_Idx] >= 0 && _STD in_range(_Exts[_Idx]), @@ -232,7 +225,7 @@ public: if constexpr (rank() != sizeof...(_OtherExtents)) { return false; } else { - for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { + for (rank_type _Idx = 0; _Idx < _Rank; ++_Idx) { if (_STD cmp_not_equal(_Left.extent(_Idx), _Right.extent(_Idx))) { return false; } @@ -253,7 +246,7 @@ public: // TRANSITION, LWG ISSUE? I believe that this function should return 'index_type' _NODISCARD constexpr index_type _Rev_prod_of_extents(const rank_type _Idx) const noexcept { index_type _Result = 1; - for (rank_type _Dim = _Idx + 1; _Dim < rank(); ++_Dim) { + for (rank_type _Dim = _Idx + 1; _Dim < _Rank; ++_Dim) { _Result *= extent(_Dim); } return _Result; @@ -361,7 +354,7 @@ public: const bool _Verify = [&](index_sequence<_Indices...>) { index_type _Prod = 1; return ((_Other.stride(_Indices) - == (_Indices == extents_type::rank() - 1 + == (_Indices == extents_type::_Rank - 1 ? _Prod : _STD exchange(_Prod, static_cast(_Prod * _Exts.extent(_Indices))))) && ...); @@ -382,7 +375,7 @@ public: } _NODISCARD constexpr index_type required_span_size() const noexcept { - return _Exts._Fwd_prod_of_extents(extents_type::rank()); + return _Exts._Fwd_prod_of_extents(extents_type::_Rank); } template @@ -419,7 +412,7 @@ public: _NODISCARD constexpr index_type stride(const rank_type _Idx) const noexcept requires (extents_type::rank() > 0) { - _STL_VERIFY(_Idx < extents_type::rank(), + _STL_VERIFY(_Idx < extents_type::_Rank, "Value of i must be less than extents_type::rank() (N4944 [mdspan.layout.left.obs]/6)."); return _Exts._Fwd_prod_of_extents(_Idx); } @@ -494,7 +487,7 @@ public: index_type _Prod = stride(0); return ( (_Other.stride(_Indices) - == (_Indices == extents_type::rank() - 1 + == (_Indices == extents_type::_Rank - 1 ? _Prod : _STD exchange(_Prod, static_cast(_Prod / _Exts.extent(_Indices + 1))))) && ...); @@ -515,7 +508,7 @@ public: } _NODISCARD constexpr index_type required_span_size() const noexcept { - return _Exts._Fwd_prod_of_extents(extents_type::rank()); + return _Exts._Fwd_prod_of_extents(extents_type::_Rank); } template @@ -552,7 +545,7 @@ public: _NODISCARD constexpr index_type stride(const rank_type _Idx) const noexcept requires (extents_type::rank() > 0) { - _STL_VERIFY(_Idx < extents_type::rank(), + _STL_VERIFY(_Idx < extents_type::_Rank, "Value of i must be less than extents_type::rank() (N4944 [mdspan.layout.right.obs]/6)."); return _Exts._Rev_prod_of_extents(_Idx); } @@ -829,11 +822,11 @@ public: "[mdspan.mdspan.overview]/2.3)."); _NODISCARD static constexpr rank_type rank() noexcept { - return extents_type::rank(); + return extents_type::_Rank; } _NODISCARD static constexpr rank_type rank_dynamic() noexcept { - return extents_type::rank_dynamic(); + return extents_type::_Rank_dynamic; } _NODISCARD static constexpr size_t static_extent(const rank_type _Rank) noexcept { @@ -949,14 +942,14 @@ public: _NODISCARD constexpr size_type size() const noexcept { const auto& _Ext = _Map.extents(); size_type _Result = 1; - for (rank_type _Dim = 0; _Dim < rank(); ++_Dim) { + for (rank_type _Dim = 0; _Dim < extents_type::_Rank; ++_Dim) { _Result *= _Ext.extent(_Dim); } return _Result; } _NODISCARD constexpr bool empty() const noexcept { - for (rank_type _Dim = 0; _Dim < rank(); ++_Dim) { + for (rank_type _Dim = 0; _Dim < extents_type::_Rank; ++_Dim) { if (_Map.extents().extent(_Dim) == 0) { return true; } From a0c173aadc8dd843a443c3a294c8636ed9cb586d Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Thu, 27 Apr 2023 01:21:05 +0200 Subject: [PATCH 2/3] Use even more _Rank --- stl/inc/mdspan | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/stl/inc/mdspan b/stl/inc/mdspan index 06230c310c5..ff1766909f1 100644 --- a/stl/inc/mdspan +++ b/stl/inc/mdspan @@ -599,7 +599,7 @@ public: constexpr mapping() noexcept : _Exts(extents_type{}) { if constexpr (extents_type::rank() != 0) { _Strides.back() = 1; - for (rank_type _Idx = extents_type::rank() - 1; _Idx-- > 0;) { + for (rank_type _Idx = extents_type::_Rank - 1; _Idx-- > 0;) { // TRANSITION USE `_Multiply_with_overflow_check` IN DEBUG MODE _Strides[_Idx] = _Strides[_Idx + 1] * _Exts.extent(_Idx + 1); } @@ -615,7 +615,7 @@ public: constexpr mapping(const extents_type& _Exts_, span<_OtherIndexType, extents_type::rank()> _Strides_, index_sequence<_Indices...>) noexcept : _Exts(_Exts_), _Strides{static_cast(_STD as_const(_Strides_[_Indices]))...} { - for (rank_type _Idx = 0; _Idx < extents_type::rank(); ++_Idx) { + for (rank_type _Idx = 0; _Idx < extents_type::_Rank; ++_Idx) { // TRANSITION CHECK [mdspan.layout.stride.cons]/4.2 (REQUIRES `_Multiply_with_overflow_check`) _STL_VERIFY(_Strides[_Idx] > 0, "Value of s[i] must be greater than 0 for all i in the range [0, rank_) " "(N4944 [mdspan.layout.stride.cons]/4.1)."); @@ -664,7 +664,7 @@ public: "[mdspan.layout.stride.cons]/7.3)."); _STL_VERIFY( _Offset(_Other) == 0, "Value of OFFSET(other) must be equal to 0 (N4944 [mdspan.layout.stride.cons]/7.4)."); - for (rank_type _Idx = 0; _Idx < extents_type::rank(); ++_Idx) { + for (rank_type _Idx = 0; _Idx < extents_type::_Rank; ++_Idx) { const auto _Stride = _Other.stride(_Idx); _STL_VERIFY(_Stride > 0, "Value of other.stride(r) must be greater than 0 for every rank index r of " "extents() (N4944 [mdspan.layout.stride.cons]/7.2)."); @@ -689,7 +689,7 @@ public: return 1; } else { index_type _Result = 1; - for (rank_type _Idx = 0; _Idx < extents_type::rank(); ++_Idx) { + for (rank_type _Idx = 0; _Idx < extents_type::_Rank; ++_Idx) { const index_type _Ext = _Exts.extent(_Idx); if (_Ext == 0) { return 0; @@ -730,7 +730,7 @@ public: if constexpr (extents_type::rank() == 0) { return true; } else { - return required_span_size() == _Exts._Fwd_prod_of_extents(extents_type::rank()); + return required_span_size() == _Exts._Fwd_prod_of_extents(extents_type::_Rank); } } @@ -751,7 +751,7 @@ public: return false; } - for (rank_type _Idx = 0; _Idx < extents_type::rank(); ++_Idx) { + for (rank_type _Idx = 0; _Idx < extents_type::_Rank; ++_Idx) { if (_Left.stride(_Idx) != _Right.stride(_Idx)) { return false; } @@ -770,7 +770,7 @@ private: if constexpr (extents_type::rank() == 0) { return _Mapping(); } else { - for (rank_type _Idx = 0; _Idx < extents_type::rank(); ++_Idx) { + for (rank_type _Idx = 0; _Idx < extents_type::_Rank; ++_Idx) { if (_Mapping.extents().extent(_Idx) == 0) { return 0; } @@ -949,11 +949,11 @@ public: } _NODISCARD constexpr size_type size() const noexcept { - return static_cast(_Map.extents()._Fwd_prod_of_extents(rank())); + return static_cast(_Map.extents()._Fwd_prod_of_extents(extents_type::_Rank)); } _NODISCARD constexpr bool empty() const noexcept { - for (rank_type _Idx = 0; _Idx < rank(); ++_Idx) { + for (rank_type _Idx = 0; _Idx < extents_type::_Rank; ++_Idx) { if (_Map.extents().extent(_Idx) == 0) { return true; } From f60bbc88285426252a47995562899dca86cda7be Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Thu, 27 Apr 2023 01:22:52 +0200 Subject: [PATCH 3/3] Address comments from previous PR --- tests/std/tests/P0009R18_mdspan_layout_stride/test.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/std/tests/P0009R18_mdspan_layout_stride/test.cpp b/tests/std/tests/P0009R18_mdspan_layout_stride/test.cpp index 57683fd893c..960c7fe4e69 100644 --- a/tests/std/tests/P0009R18_mdspan_layout_stride/test.cpp +++ b/tests/std/tests/P0009R18_mdspan_layout_stride/test.cpp @@ -15,7 +15,7 @@ using namespace std; template constexpr void do_check_members(const extents& ext, - const array strs, index_sequence) { + const array& strs, index_sequence) { using Ext = extents; using Strides = array; using Mapping = layout_stride::mapping; @@ -116,7 +116,7 @@ constexpr void do_check_members(const extents& ext, // Tests of 'is_exhaustive' are defined in 'check_is_exhaustive' function [FIXME] } - { // Check 'stride' function (intentionally not if constexpr) + { // Check 'stride' function for (size_t i = 0; i < strs.size(); ++i) { same_as decltype(auto) s = m.stride(i); assert(strs[i] == s); @@ -132,7 +132,7 @@ constexpr void do_check_members(const extents& ext, } template -constexpr void check_members(extents ext, const array strides) { +constexpr void check_members(extents ext, const array& strides) { do_check_members(ext, strides, make_index_sequence{}); }