Skip to content

<mdspan>: extents improvements #3786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
StephanTLavavej merged 13 commits into from
Jun 21, 2023
Merged

<mdspan>: extents improvements #3786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
3177a96
Implement extra internal checks in `_(Fwd/Rev)_prod_of_extents` and:
JMazurkiewicz May 28, 2023
2248ae7
Make `extents(array/span)` constructor a little bit more efficient an…
JMazurkiewicz May 28, 2023
e76155d
Improve checks in `extents(indices...)` constructor
JMazurkiewicz Jun 16, 2023
9f980f4
Drive-by: add parentheses around `std::min` in `test_mdspan_support.hpp`
JMazurkiewicz Jun 19, 2023
56a1058
Improve `extent(other extents)` constructor
JMazurkiewicz Jun 19, 2023
0f22be9
Improve `extents(indices...)` constructor
JMazurkiewicz Jun 19, 2023
d20371e
Avoid another `for` loop
JMazurkiewicz Jun 19, 2023
2f587d5
Use `_CONTAINER_DEBUG_LEVEL` to guard debug checks
JMazurkiewicz Jun 19, 2023
fb0c9aa
Dead code: integer-like classes cannot be used as an argument to `ext…
JMazurkiewicz Jun 19, 2023
8e777be
Extra death test for `extents`
JMazurkiewicz Jun 19, 2023
d3cff2d
Add explicit defaulted constructor to `_Construct_from_tuple` tag type
JMazurkiewicz Jun 19, 2023
b9ff051
Remove unnecessary members of `_Static_extents_only` type
JMazurkiewicz Jun 19, 2023
ed8b639
Poke CI
JMazurkiewicz Jun 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 91 additions & 63 deletions stl/inc/mdspan
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ _EMIT_STL_WARNING(STL4038, "The contents of <mdspan> are available only with C++
#include <array>
#include <limits>
#include <span>
#include <tuple>
#include <type_traits>

#pragma pack(push, _CRT_PACKING)
Expand Down Expand Up @@ -74,15 +75,12 @@ private:

static constexpr array<rank_type, _Rank> _Dynamic_indices_inv = _Make_dynamic_indices_inv();

struct _Construct_from_tuple {
constexpr explicit _Construct_from_tuple() noexcept = default;
};

struct _Static_extents_only {
constexpr explicit _Static_extents_only() noexcept = default;

template <class... _Args>
constexpr explicit _Static_extents_only(_Args&&...) noexcept {}

_NODISCARD constexpr index_type* begin() const noexcept {
return nullptr;
}
};

conditional_t<_Rank_dynamic != 0, array<index_type, _Rank_dynamic>, _Static_extents_only> _Dynamic_extents{};
Expand All @@ -97,12 +95,16 @@ public:
}

_NODISCARD static constexpr size_t static_extent(const rank_type _Idx) noexcept {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Idx < _Rank, "Index must be less than rank() (N4950 [mdspan.extents.obs]/1)");
#endif // _CONTAINER_DEBUG_LEVEL > 0
return _Static_extents[_Idx];
}

_NODISCARD constexpr index_type extent(const rank_type _Idx) const noexcept {
#if _CONTAINER_DEBUG_LEVEL > 0
_STL_VERIFY(_Idx < _Rank, "Index must be less than rank() (N4950 [mdspan.extents.obs]/3)");
#endif // _CONTAINER_DEBUG_LEVEL > 0
if constexpr (rank_dynamic() == 0) {
return static_cast<index_type>(_Static_extents[_Idx]);
} else if constexpr (rank_dynamic() == rank()) {
Expand All @@ -118,88 +120,110 @@ public:

constexpr extents() noexcept = default;

template <class _OtherIndexType, size_t... _OtherExtents, size_t... _Indices>
requires (sizeof...(_OtherExtents) == rank())
&& ((_OtherExtents == dynamic_extent || _Extents == dynamic_extent || _OtherExtents == _Extents) && ...)
constexpr explicit extents(
const extents<_OtherIndexType, _OtherExtents...>& _Other, index_sequence<_Indices...>) noexcept
: _Dynamic_extents{static_cast<index_type>(_Other.extent(_Dynamic_indices_inv[_Indices]))...} {
#if _CONTAINER_DEBUG_LEVEL > 0
if constexpr (rank() > 0) {
for (rank_type _Idx = 0; _Idx < _Rank; ++_Idx) {
if constexpr (rank() != rank_dynamic()) {
_STL_VERIFY(_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 (N4950 [mdspan.extents.cons]/2.1)");
}
_STL_VERIFY(_STD in_range<index_type>(_Other.extent(_Idx)),
"Value of other.extent(r) must be representable as a value of type index_type for every rank index "
"r (N4950 [mdspan.extents.cons]/2.2)");
}
}
#endif // _CONTAINER_DEBUG_LEVEL > 0
}

template <class _OtherIndexType, size_t... _OtherExtents>
requires (sizeof...(_OtherExtents) == rank())
&& ((_OtherExtents == dynamic_extent || _Extents == dynamic_extent || _OtherExtents == _Extents) && ...)
constexpr explicit(((_Extents != dynamic_extent && _OtherExtents == dynamic_extent) || ...)
|| (numeric_limits<index_type>::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) {
_STL_VERIFY(
_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 "
"(N4950 [mdspan.extents.cons]/2.1)");
_STL_VERIFY(_STD in_range<index_type>(_Other.extent(_Idx)),
"Value of other.extent(r) must be representable as a value of type index_type for every rank index r "
"(N4950 [mdspan.extents.cons]/2.2)");

if (_Static_extents[_Idx] == dynamic_extent) {
*_It = static_cast<index_type>(_Other.extent(_Idx));
++_It;
}
}
extents(const extents<_OtherIndexType, _OtherExtents...>& _Other) noexcept
: extents(_Other, make_index_sequence<rank_dynamic()>{}) {}

template <class _ExtsTuple, size_t... _Indices>
requires (tuple_size_v<_ExtsTuple> == rank_dynamic())
constexpr explicit extents(_Construct_from_tuple, _ExtsTuple _Tpl, index_sequence<_Indices...>) noexcept
: _Dynamic_extents{static_cast<index_type>(_STD move(_STD get<_Indices>(_Tpl)))...} {}

template <class _ExtsTuple, size_t... _DynIndices>
requires (tuple_size_v<_ExtsTuple> != rank_dynamic())
constexpr explicit extents(_Construct_from_tuple, _ExtsTuple _Tpl, index_sequence<_DynIndices...>) noexcept
: _Dynamic_extents{static_cast<index_type>(_STD move(_STD get<_Dynamic_indices_inv[_DynIndices]>(_Tpl)))...} {
#if _CONTAINER_DEBUG_LEVEL > 0
[&]<size_t... _MixedIndices>(index_sequence<_MixedIndices...>) {
_STL_VERIFY(((_Static_extents[_MixedIndices] == dynamic_extent
|| _STD cmp_equal(_Static_extents[_MixedIndices],
static_cast<index_type>(_STD move(_STD get<_MixedIndices>(_Tpl)))))
&& ...),
"Value of exts_arr[r] must be equal to extent(r) for each r for which extent(r) is a static extent "
"(N4950 [mdspan.extents.cons]/7.1)");
}(make_index_sequence<rank()>{});
#endif // _CONTAINER_DEBUG_LEVEL > 0
}

template <class... _OtherIndexTypes>
requires (is_convertible_v<_OtherIndexTypes, index_type> && ...)
&& (is_nothrow_constructible_v<index_type, _OtherIndexTypes> && ...)
&& (sizeof...(_OtherIndexTypes) == rank_dynamic() || sizeof...(_OtherIndexTypes) == rank())
constexpr explicit extents(_OtherIndexTypes... _Exts) noexcept {
if constexpr ((_Is_standard_integer<_OtherIndexTypes> && ...)) {
_STL_VERIFY(sizeof...(_Exts) == 0 || ((_Exts >= 0 && _STD in_range<index_type>(_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 (N4950 [mdspan.extents.cons]/7.2)");
}

if constexpr (sizeof...(_Exts) == rank_dynamic()) {
_Dynamic_extents = {static_cast<index_type>(_STD move(_Exts))...};
} else {
array<index_type, sizeof...(_Exts)> _Exts_arr{static_cast<index_type>(_STD move(_Exts))...};
auto _It = _Dynamic_extents.begin();
for (rank_type _Idx = 0; _Idx < _Rank; ++_Idx) {
_STL_VERIFY(
_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 "
"(N4950 [mdspan.extents.cons]/7.1)");
if (_Static_extents[_Idx] == dynamic_extent) {
*_It = _Exts_arr[_Idx];
++_It;
}
constexpr explicit extents(_OtherIndexTypes... _Exts) noexcept
: extents(_Construct_from_tuple{}, _STD tie(_Exts...), make_index_sequence<rank_dynamic()>{}) {
#if _CONTAINER_DEBUG_LEVEL > 0
auto _Check_extent = []<class _Ty>(const _Ty& _Ext) {
if constexpr (_Is_standard_integer<_Ty>) {
return _Ext >= 0 && _STD in_range<index_type>(_Ext);
} else {
return true; // NB: We cannot check preconditions
}
}
};
_STL_VERIFY((_Check_extent(_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 (N4950 [mdspan.extents.cons]/7.2)");
#endif // _CONTAINER_DEBUG_LEVEL > 0
}

template <class _OtherIndexType, size_t _Size, size_t... _Indices>
requires is_convertible_v<const _OtherIndexType&, index_type>
&& is_nothrow_constructible_v<index_type, const _OtherIndexType&> && (_Size != rank())
constexpr explicit extents(span<_OtherIndexType, _Size> _Exts, index_sequence<_Indices...>) noexcept
: _Dynamic_extents{static_cast<index_type>(_STD as_const(_Exts[_Indices]))...} {
&& is_nothrow_constructible_v<index_type, const _OtherIndexType&> && (_Size == rank_dynamic())
constexpr explicit extents(span<_OtherIndexType, _Size> _Dynamic_exts, index_sequence<_Indices...>) noexcept
: _Dynamic_extents{static_cast<index_type>(_STD as_const(_Dynamic_exts[_Indices]))...} {
#if _CONTAINER_DEBUG_LEVEL > 0
if constexpr (_Is_standard_integer<_OtherIndexType> && _Size != 0) {
for (_OtherIndexType _Ext : _Exts) {
_STL_VERIFY(_Ext >= 0 && _STD in_range<index_type>(_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 (N4950 [mdspan.extents.cons]/10.2)");
}
_STL_VERIFY(((_Dynamic_exts[_Indices] >= 0 && _STD in_range<index_type>(_Dynamic_exts[_Indices])) && ...),
"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 (N4950 [mdspan.extents.cons]/10.2)");
}
#endif // _CONTAINER_DEBUG_LEVEL > 0
}

template <class _OtherIndexType, size_t _Size, size_t... _Indices>
requires is_convertible_v<const _OtherIndexType&, index_type>
&& is_nothrow_constructible_v<index_type, const _OtherIndexType&> && (_Size == rank())
constexpr explicit extents(span<_OtherIndexType, _Size> _Exts, index_sequence<_Indices...>) noexcept
: _Dynamic_extents{static_cast<index_type>(_STD as_const(_Exts[_Dynamic_indices_inv[_Indices]]))...} {
&& is_nothrow_constructible_v<index_type, const _OtherIndexType&> && (_Size != rank_dynamic())
constexpr explicit extents(span<_OtherIndexType, _Size> _Mixed_exts, index_sequence<_Indices...>) noexcept
: _Dynamic_extents{static_cast<index_type>(_STD as_const(_Mixed_exts[_Dynamic_indices_inv[_Indices]]))...} {
#if _CONTAINER_DEBUG_LEVEL > 0
if constexpr (_Is_standard_integer<_OtherIndexType>) {
for (rank_type _Idx = 0; _Idx < _Rank; ++_Idx) {
_STL_VERIFY(
_Static_extents[_Idx] == dynamic_extent || _STD cmp_equal(_Static_extents[_Idx], _Exts[_Idx]),
_Static_extents[_Idx] == dynamic_extent || _STD cmp_equal(_Static_extents[_Idx], _Mixed_exts[_Idx]),
"Value of exts[r] must be equal to extent(r) for each r for which extent(r) is a static extent "
"(N4950 [mdspan.extents.cons]/10.1)");
_STL_VERIFY(_Exts[_Idx] >= 0 && _STD in_range<index_type>(_Exts[_Idx]),
_STL_VERIFY(_Mixed_exts[_Idx] >= 0 && _STD in_range<index_type>(_Mixed_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 (N4950 [mdspan.extents.cons]/10.2)");
}
}
#endif // _CONTAINER_DEBUG_LEVEL > 0
}

template <class _OtherIndexType, size_t _Size>
Expand Down Expand Up @@ -280,6 +304,7 @@ template <class _Extents>
class _Fwd_prod_of_extents {
public:
_NODISCARD static constexpr _Extents::index_type _Calculate(const _Extents& _Exts, const size_t _Idx) noexcept {
_STL_INTERNAL_CHECK(_Idx <= _Extents::_Rank);
if constexpr (_Extents::rank() == 0) {
return 1;
} else {
Expand All @@ -293,16 +318,16 @@ public:
};

template <class _IndexType, size_t... _Extents>
requires ((_Extents != dynamic_extent) && ...)
requires (sizeof...(_Extents) > 0) && ((_Extents != dynamic_extent) && ...)
class _Fwd_prod_of_extents<extents<_IndexType, _Extents...>> {
private:
using _Ty = extents<_IndexType, _Extents...>;

_NODISCARD static consteval auto _Make_prods() noexcept {
array<typename _Ty::index_type, _Ty::rank() + 1> _Result;
_Result.front() = 1;
for (size_t _Dim = 1; _Dim < _Ty::_Rank + 1; ++_Dim) {
_Result[_Dim] = static_cast<_Ty::index_type>(_Result[_Dim - 1] * _Ty::static_extent(_Dim - 1));
for (size_t _Idx = 1; _Idx < _Ty::_Rank + 1; ++_Idx) {
_Result[_Idx] = static_cast<_Ty::index_type>(_Result[_Idx - 1] * _Ty::static_extent(_Idx - 1));
}
return _Result;
}
Expand All @@ -311,6 +336,7 @@ private:

public:
_NODISCARD static constexpr _Ty::index_type _Calculate(const _Ty&, const size_t _Idx) noexcept {
_STL_INTERNAL_CHECK(_Idx <= _Ty::_Rank);
return _Cache[_Idx];
}
};
Expand All @@ -320,6 +346,7 @@ template <class _Extents>
class _Rev_prod_of_extents {
public:
_NODISCARD static constexpr _Extents::index_type _Calculate(const _Extents& _Exts, const size_t _Idx) noexcept {
_STL_INTERNAL_CHECK(_Idx < _Extents::_Rank);
typename _Extents::index_type _Result = 1;
for (size_t _Dim = _Idx + 1; _Dim < _Extents::_Rank; ++_Dim) {
_Result *= _Exts.extent(_Dim);
Expand All @@ -337,8 +364,8 @@ private:
_NODISCARD static consteval auto _Make_prods() noexcept {
array<typename _Ty::index_type, _Ty::rank()> _Result;
_Result.back() = 1;
for (size_t _Dim = _Ty::_Rank; _Dim-- > 1;) {
_Result[_Dim - 1] = static_cast<_Ty::index_type>(_Result[_Dim] * _Ty::static_extent(_Dim));
for (size_t _Idx = _Ty::_Rank; _Idx-- > 1;) {
_Result[_Idx - 1] = static_cast<_Ty::index_type>(_Result[_Idx] * _Ty::static_extent(_Idx));
}
return _Result;
}
Expand All @@ -347,6 +374,7 @@ private:

public:
_NODISCARD static constexpr _Ty::index_type _Calculate(const _Ty&, const size_t _Idx) noexcept {
_STL_INTERNAL_CHECK(_Idx < _Ty::_Rank);
return _Cache[_Idx];
}
};
Expand Down
4 changes: 2 additions & 2 deletions tests/std/include/test_mdspan_support.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ namespace details {
template <size_t... Extents, class Fn>
constexpr void check_members_with_mixed_extents(Fn&& fn) {
auto select_extent = [](size_t e) consteval {
return e == std::dynamic_extent ? std::min(sizeof...(Extents), size_t{3}) : e;
return e == std::dynamic_extent ? (std::min)(sizeof...(Extents), size_t{3}) : e;
};

// Check signed integers
Expand All @@ -198,7 +198,7 @@ namespace details {
template <class Fn, size_t... Seq>
constexpr void check_members_with_various_extents_impl(Fn&& fn, std::index_sequence<Seq...>) {
auto static_or_dynamic = [](size_t i) consteval {
return i == 0 ? std::dynamic_extent : std::min(sizeof...(Seq), size_t{3});
return i == 0 ? std::dynamic_extent : (std::min)(sizeof...(Seq), size_t{3});
};

if constexpr (sizeof...(Seq) <= 1) {
Expand Down
14 changes: 12 additions & 2 deletions tests/std/tests/P0009R18_mdspan_extents_death/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
// Copyright (c) Microsoft Corporation.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#define _CONTAINER_DEBUG_LEVEL 1

#include <array>
#include <cstddef>
#include <mdspan>
#include <span>

#include <test_death.hpp>
#include <test_mdspan_support.hpp>

using namespace std;

Expand Down Expand Up @@ -39,12 +42,18 @@ void test_construction_from_pack_with_invalid_values() {
[[maybe_unused]] extents<int, 1, 2> e{1, 1};
}

void test_construction_from_pack_with_unrepresentable_as_index_type_values() {
void test_construction_from_pack_with_unrepresentable_as_index_type_values_1() {
// 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<unsigned char, 1, dynamic_extent> e{1, 256};
}

void test_construction_from_pack_with_unrepresentable_as_index_type_values_2() {
// 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<unsigned char, 1, dynamic_extent> e{ConvertibleToInt<unsigned char>{.val = 1}, 256};
}

void test_construction_from_span_with_invalid_values() {
int vals[] = {1, 2};
span s{vals};
Expand Down Expand Up @@ -82,7 +91,8 @@ int main(int argc, char* argv[]) {
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_pack_with_unrepresentable_as_index_type_values_1,
test_construction_from_pack_with_unrepresentable_as_index_type_values_2,
test_construction_from_span_with_invalid_values,
test_construction_from_span_with_unrepresentable_as_index_type_values,
test_construction_from_array_with_invalid_values,
Expand Down