Skip to content

Commit 5459853

Browse files
Implement LWG-4266: layout_stride::mapping should treat empty mappings as exhaustive (#5880)
Co-authored-by: Stephan T. Lavavej <[email protected]>
1 parent bbe7079 commit 5459853

File tree

3 files changed

+47
-88
lines changed

3 files changed

+47
-88
lines changed

stl/inc/mdspan

Lines changed: 24 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ public:
316316
}
317317

318318
template <class _ExtentsT>
319-
friend constexpr pair<size_t, size_t> _Count_dynamic_extents_equal_to_zero_or_one(
319+
friend constexpr size_t _Count_dynamic_extents_equal_to_one(
320320
const _ExtentsT&) noexcept; // NB: used by 'layout_stride::mapping<E>::is_exhaustive'
321321
};
322322

@@ -797,26 +797,10 @@ concept _Layout_mapping_alike = requires {
797797
bool_constant<_Mp::is_always_unique()>::value;
798798
};
799799

800-
template <class _Extents, size_t _Val>
801-
constexpr size_t _Count_static_extents_equal_to = 0;
802-
803-
template <class _IndexType, size_t... _Extents, size_t _Val>
804-
constexpr size_t _Count_static_extents_equal_to<extents<_IndexType, _Extents...>, _Val> =
805-
(static_cast<size_t>(_Extents == _Val) + ... + 0);
806-
807800
template <class _Extents>
808-
_NODISCARD constexpr pair<size_t, size_t> _Count_dynamic_extents_equal_to_zero_or_one(const _Extents& _Exts) noexcept {
801+
_NODISCARD constexpr size_t _Count_dynamic_extents_equal_to_one(const _Extents& _Exts) noexcept {
809802
_STL_INTERNAL_STATIC_ASSERT(_Is_extents<_Extents> && _Extents::rank_dynamic() != 0);
810-
size_t _Zero_extents = 0;
811-
size_t _One_extents = 0;
812-
for (const auto& _Ext : _Exts._Array) {
813-
if (_Ext == 0) {
814-
++_Zero_extents;
815-
} else if (_Ext == 1) {
816-
++_One_extents;
817-
}
818-
}
819-
return {_Zero_extents, _One_extents};
803+
return static_cast<size_t>(_RANGES count(_Exts._Array, static_cast<_Extents::index_type>(1)));
820804
}
821805

822806
template <class _Extents>
@@ -833,7 +817,7 @@ public:
833817
private:
834818
using _Extents_base = _Maybe_fully_static_extents<extents_type>;
835819
using _Strides_base = _Maybe_empty_array<index_type, _Extents::rank()>;
836-
using _Stride_extent_pair = pair<rank_type, rank_type>;
820+
using _Stride_extent_pair = pair<index_type, index_type>;
837821

838822
static_assert(_Is_extents<extents_type>,
839823
"Extents must be a specialization of std::extents (N4950 [mdspan.layout.stride.overview]/2).");
@@ -869,7 +853,7 @@ private:
869853
|| _Add_overflow(_Req_span_size, _Prod, _Req_span_size);
870854
}
871855

872-
_Pairs[_Idx] = {static_cast<rank_type>(_Stride), static_cast<rank_type>(_Ext)};
856+
_Pairs[_Idx] = {_Stride, _Ext};
873857
}
874858
_STL_VERIFY(_Found_zero || !_Overflow, "REQUIRED-SPAN-SIZE(e, s) must be representable as a value of type "
875859
"index_type (N4950 [mdspan.layout.stride.cons]/4.2).");
@@ -998,7 +982,7 @@ public:
998982
}
999983

1000984
_NODISCARD static constexpr bool is_always_exhaustive() noexcept {
1001-
return false;
985+
return extents_type::_Rank == 0 || extents_type::_Multidim_index_space_size_is_always_zero;
1002986
}
1003987

1004988
_NODISCARD static constexpr bool is_always_strided() noexcept {
@@ -1010,53 +994,28 @@ public:
1010994
}
1011995

1012996
_NODISCARD constexpr bool is_exhaustive() const noexcept {
1013-
constexpr size_t _Static_zero_extents = _Count_static_extents_equal_to<extents_type, 0>;
1014-
if constexpr (extents_type::rank() == 0) {
997+
if constexpr (is_always_exhaustive()) {
1015998
return true;
1016999
} else if constexpr (extents_type::rank() == 1) {
1017-
return this->_Array[0] == 1;
1018-
} else if constexpr (_Static_zero_extents >= 2) {
1019-
// Per N5008 [mdspan.layout.stride.obs]/5.2, we are looking for a permutation P of integers in the range
1020-
// '[0, rank)' such that 'stride(p[i]) == stride(p[i-1])*extent(p[i-1])' is true for 'i' in the range
1021-
// '[1, rank)'. Knowing that at least two extents are equal to zero, we can deduce that such a permutation
1022-
// does not exist:
1023-
// - Some 'stride(p[j])' would have to be equal to 'stride(p[j-1])*extents(p[j-1]) = stride(p[j-1])*0 = 0'
1024-
// which is not possible.
1025-
// - Only 'extent(p[rank-1])' can be equal to 0, because it's not required to satisfy the condition above.
1026-
// Since we have two or more extents equal to 0 this is not possible either.
1027-
return false;
1000+
return this->_Array[0] == 1 || this->_Exts.extent(0) == 0;
10281001
} else if constexpr (extents_type::rank() == 2) {
10291002
return (this->_Array[0] == 1 && this->_Array[1] == this->_Exts.extent(0))
1030-
|| (this->_Array[1] == 1 && this->_Array[0] == this->_Exts.extent(1));
1031-
} else {
1032-
// NB: Extents equal to 1 are problematic too - sometimes in such cases even when the mapping is exhaustive
1003+
|| (this->_Array[1] == 1 && this->_Array[0] == this->_Exts.extent(1)) || this->_Exts.extent(0) == 0
1004+
|| this->_Exts.extent(1) == 0;
1005+
} else if constexpr (_RANGES count(extents_type::_Static_extents, static_cast<rank_type>(1)) != 0) {
1006+
// NB: Extents equal to 1 are problematic - sometimes in such cases even when the mapping is exhaustive
10331007
// this function should return false.
10341008
// For example, when the extents are [2, 1, 2] and the strides are [1, 5, 2], the mapping is exhaustive
10351009
// per N5008 [mdspan.layout.reqmts]/16 but not per N5008 [mdspan.layout.stride.obs]/5.2.
1036-
constexpr size_t _Static_zero_or_one_extents =
1037-
_Static_zero_extents + _Count_static_extents_equal_to<extents_type, 1>;
1038-
1039-
if constexpr (extents_type::rank_dynamic() != 0) {
1040-
const auto [_Dynamic_zero_extents, _Dynamic_one_extents] =
1041-
_Count_dynamic_extents_equal_to_zero_or_one(this->_Exts);
1042-
1043-
const size_t _All_zero_extents = _Static_zero_extents + _Dynamic_zero_extents;
1044-
if (_All_zero_extents >= 2) {
1045-
return false;
1046-
}
1047-
1048-
const size_t _All_zero_or_one_extents =
1049-
_Static_zero_or_one_extents + _Dynamic_zero_extents + _Dynamic_one_extents;
1050-
if (_All_zero_or_one_extents == 0) {
1051-
return _Is_exhaustive_common_case();
1052-
}
1053-
1054-
return _Is_exhaustive_special_case();
1055-
} else if constexpr (_Static_zero_or_one_extents == 0) {
1056-
return _Is_exhaustive_common_case();
1057-
} else {
1010+
return _Is_exhaustive_special_case();
1011+
} else if constexpr (extents_type::rank_dynamic() == 0) {
1012+
return _Is_exhaustive_common_case();
1013+
} else {
1014+
if (_Count_dynamic_extents_equal_to_one(this->_Exts) != 0) {
10581015
return _Is_exhaustive_special_case();
10591016
}
1017+
1018+
return _Is_exhaustive_common_case();
10601019
}
10611020
}
10621021

@@ -1127,20 +1086,19 @@ private:
11271086
}
11281087

11291088
_NODISCARD constexpr bool _Is_exhaustive_common_case() const noexcept {
1130-
return required_span_size() == _Fwd_prod_of_extents<extents_type>::_Calculate(this->_Exts, extents_type::_Rank);
1089+
const index_type _Prod = _Fwd_prod_of_extents<extents_type>::_Calculate(this->_Exts, extents_type::_Rank);
1090+
return _Prod == required_span_size() || _Prod == 0;
11311091
}
11321092

11331093
_NODISCARD constexpr bool _Is_exhaustive_special_case() const noexcept {
11341094
array<_Stride_extent_pair, extents_type::rank()> _Pairs;
11351095
for (rank_type _Idx = 0; _Idx < extents_type::_Rank; ++_Idx) {
1136-
rank_type _Ext = static_cast<rank_type>(this->_Exts.extent(_Idx));
1096+
const index_type _Ext = this->_Exts.extent(_Idx);
11371097
if (_Ext == 0) {
1138-
// NB: _Ext equal to zero is special - we want it to end up as close to the end of the sorted range as
1139-
// possible, so we assign max value of rank_type to it.
1140-
_Ext = static_cast<rank_type>(-1);
1098+
return true;
11411099
}
11421100

1143-
_Pairs[_Idx] = {static_cast<rank_type>(this->_Array[_Idx]), _Ext};
1101+
_Pairs[_Idx] = {this->_Array[_Idx], _Ext};
11441102
}
11451103

11461104
_RANGES sort(_Pairs);

tests/libcxx/expected_results.txt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,12 +181,13 @@ std/ranges/range.adaptors/range.lazy.split/range.lazy.split.outer.value/ctor.ite
181181
# libc++ doesn't implement LWG-4112
182182
std/ranges/range.adaptors/range.join/range.join.iterator/arrow.pass.cpp FAIL
183183

184+
# libc++ doesn't implement LWG-4266 "`layout_stride::mapping` should treat empty mappings as exhaustive"
185+
std/containers/views/mdspan/layout_stride/is_exhaustive_corner_case.pass.cpp FAIL
186+
std/containers/views/mdspan/layout_stride/properties.pass.cpp FAIL
187+
184188
# If any feature-test macro test is failing, this consolidated test will also fail.
185189
std/language.support/support.limits/support.limits.general/version.version.compile.pass.cpp FAIL
186190

187-
# libc++ incorrectly implements `layout_stride::mapping<E>::is_exhaustive()`
188-
std/containers/views/mdspan/layout_stride/is_exhaustive_corner_case.pass.cpp FAIL
189-
190191

191192
# *** INTERACTIONS WITH MSVC THAT UPSTREAM LIKELY WON'T FIX ***
192193
# These tests set an allocator with a max_size() too small to default construct an unordered container

tests/std/tests/P0009R18_mdspan_layout_stride/test.cpp

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ constexpr void do_check_members(const extents<IndexType, Extents...>& ext,
199199

200200
{ // Check 'is_always_[unique/exhaustive/strided]' functions
201201
static_assert(Mapping::is_always_unique());
202-
static_assert(!Mapping::is_always_exhaustive());
202+
static_assert(Mapping::is_always_exhaustive() == (Ext::rank() == 0 || ((Extents == 0) || ...)));
203203
static_assert(Mapping::is_always_strided());
204204
}
205205

@@ -410,7 +410,7 @@ constexpr void check_is_exhaustive() {
410410

411411
// rank() is equal to 1
412412
check(extents<int, 0>{}, array{1}, true);
413-
check(dextents<int, 1>{0}, array{2}, false);
413+
check(dextents<int, 1>{0}, array{2}, true);
414414
check(extents<int, 1>{}, array{3}, false);
415415
check(dextents<int, 1>{2}, array{2}, false);
416416
check(extents<int, 3>{}, array{1}, true);
@@ -426,14 +426,14 @@ constexpr void check_is_exhaustive() {
426426
check(extents<int, dynamic_extent, 7>{5}, array{1, 8}, false);
427427
check(dextents<int, 2>{6, 5}, array{1, 10}, false);
428428
check(extents<int, 0, 3>{}, array{3, 1}, true);
429-
check(extents<int, 0, 3>{}, array{6, 2}, false);
430-
check(extents<int, dynamic_extent, 3>{0}, array{6, 1}, false);
431-
check(extents<int, 0, dynamic_extent>{3}, array{6, 2}, false);
432-
check(dextents<int, 2>{0, 3}, array{7, 2}, false);
433-
check(extents<int, 0, 0>{}, array{1, 1}, false);
434-
check(extents<int, 0, dynamic_extent>{0, 0}, array{1, 1}, false);
435-
check(dextents<int, 2>{0, 0}, array{1, 2}, false);
436-
check(extents<int, 1, dynamic_extent>{0}, array{1, 2}, false);
429+
check(extents<int, 0, 3>{}, array{6, 2}, true);
430+
check(extents<int, dynamic_extent, 3>{0}, array{6, 1}, true);
431+
check(extents<int, 0, dynamic_extent>{3}, array{6, 2}, true);
432+
check(dextents<int, 2>{0, 3}, array{7, 2}, true);
433+
check(extents<int, 0, 0>{}, array{1, 1}, true);
434+
check(extents<int, 0, dynamic_extent>{0, 0}, array{1, 1}, true);
435+
check(dextents<int, 2>{0, 0}, array{1, 2}, true);
436+
check(extents<int, 1, dynamic_extent>{0}, array{1, 2}, true);
437437

438438
// rank() is greater than 2
439439
check(extents<int, 2, 3, 5>{}, array{1, 2, 6}, true);
@@ -449,20 +449,20 @@ constexpr void check_is_exhaustive() {
449449
// rank() is greater than 2 and some extents are equal to 0
450450
check(extents<int, 2, 0, 7>{}, array{7, 14, 1}, true);
451451
check(extents<int, dynamic_extent, 0, 7>{2}, array{1, 14, 2}, true);
452-
check(extents<int, 2, dynamic_extent, 7>{0}, array{14, 28, 1}, false);
453-
check(extents<int, 2, dynamic_extent, dynamic_extent>{0, 7}, array{1, 2, 2}, false);
454-
check(dextents<int, 3>{2, 0, 7}, array{2, 28, 4}, false);
455-
check(extents<int, 5, 0, 0>{}, array{3, 1, 1}, false);
456-
check(extents<int, 5, dynamic_extent, 0>{0}, array{1, 5, 1}, false);
457-
check(dextents<int, 3>{5, 0, 0}, array{2, 1, 10}, false);
458-
check(extents<int, 0, 0, 0>{}, array{1, 1, 1}, false);
452+
check(extents<int, 2, dynamic_extent, 7>{0}, array{14, 28, 1}, true);
453+
check(extents<int, 2, dynamic_extent, dynamic_extent>{0, 7}, array{1, 2, 2}, true);
454+
check(dextents<int, 3>{2, 0, 7}, array{2, 28, 4}, true);
455+
check(extents<int, 5, 0, 0>{}, array{3, 1, 1}, true);
456+
check(extents<int, 5, dynamic_extent, 0>{0}, array{1, 5, 1}, true);
457+
check(dextents<int, 3>{5, 0, 0}, array{2, 1, 10}, true);
458+
check(extents<int, 0, 0, 0>{}, array{1, 1, 1}, true);
459459
check(extents<int, 0, 1, 1>{}, array{1, 1, 1}, true);
460460

461461
// rank() is greater than 2 - one extent is equal to 0 while others are equal to each other
462462
check(extents<int, 3, 0, 3>{}, array{1, 9, 3}, true);
463463
check(extents<int, dynamic_extent, 0, 3>{3}, array{3, 9, 1}, true);
464-
check(extents<int, 3, dynamic_extent, dynamic_extent>{0, 3}, array{1, 3, 3}, false);
465-
check(dextents<int, 3>{3, 0, 3}, array{1, 4, 8}, false);
464+
check(extents<int, 3, dynamic_extent, dynamic_extent>{0, 3}, array{1, 3, 3}, true);
465+
check(dextents<int, 3>{3, 0, 3}, array{1, 4, 8}, true);
466466
check(dextents<int, 3>{0, 1, 1}, array{1, 1, 1}, true);
467467

468468
// required_span_size() is equal to 1

0 commit comments

Comments
 (0)