Skip to content

<mdspan>: Implement *Mandates* clauses #3535

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
Stephan T. Lavavej (StephanTLavavej) merged 6 commits into from
Mar 4, 2023
Merged

<mdspan>: Implement *Mandates* clauses #3535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8487e23
Implement *Mandates* clauses and even more cleanups!
JMazurkiewicz Mar 4, 2023
01cf15e
Add `inline` to `_Is_extents_v` variable
JMazurkiewicz Mar 4, 2023
3597d3c
But remove `_v` suffix - see [mdspan.layout.stride.expo]/3
JMazurkiewicz Mar 4, 2023
954b88c
*`is-mapping-of`* missed `inline` too
JMazurkiewicz Mar 4, 2023
5dd1358
The Show MUST Go On
JMazurkiewicz Mar 4, 2023
307bbd1
More static assertions!
JMazurkiewicz Mar 4, 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
98 changes: 69 additions & 29 deletions stl/inc/mdspan
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,11 @@ public:
using size_type = make_unsigned_t<index_type>;
using rank_type = size_t;

// TRANSITION: doesn't account for extended integer types
static_assert(_Is_any_of_v<remove_cv_t<_IndexType>, 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>,
Comment thread
JMazurkiewicz marked this conversation as resolved.
"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);
Expand Down Expand Up @@ -228,6 +226,14 @@ public:
}
}
}

_NODISCARD static constexpr bool _Is_index_space_size_representable() {
if constexpr (rank_dynamic() == 0 && rank() > 0) {
return _STD in_range<index_type>((_Extents * ...));

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) Mar 4, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to worry about integer overflow during this monster multiplication?

@MattStephanson Matt Stephanson (MattStephanson) Mar 5, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only references I could find to index_type being able to represent the index space size are in Preconditions to the various layout mapping constructors. If we're going to upgrade it to a static_assert, then I think we do need to check for overflow, because it can happen and the compiler is supposed to diagnose UB in constant evaluation contexts. We also don't want any false positives, since the user won't have any way to bypass this check.

} else {
return true;
}
}
};

template <class _IndexType, size_t _Rank>
Expand All @@ -242,10 +248,10 @@ extents(_Integrals... _Ext)
-> extents<size_t, conditional_t<true, integral_constant<size_t, dynamic_extent>, _Integrals>::value...>;

template <class _Type>
constexpr bool _Is_extents = false;
inline constexpr bool _Is_extents = false;

template <class _IndexType, size_t... _Args>
constexpr bool _Is_extents<extents<_IndexType, _Args...>> = true;
inline constexpr bool _Is_extents<extents<_IndexType, _Args...>> = true;

template <class _Mapping, class = void>
struct _Layout_mapping_alike_helper : false_type {};
Expand All @@ -262,7 +268,7 @@ template <class _Mapping>
struct _Layout_mapping_alike : bool_constant<_Layout_mapping_alike_helper<_Mapping>::value> {};

template <class _Layout, class _Mapping>
constexpr bool _Is_mapping_of =
inline constexpr bool _Is_mapping_of =
is_same_v<typename _Layout::template mapping<typename _Mapping::extents_type>, _Mapping>;

struct layout_left {
Expand All @@ -289,15 +295,21 @@ 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).");
Comment thread
JMazurkiewicz marked this conversation as resolved.
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.");
Comment thread
JMazurkiewicz marked this conversation as resolved.

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 <class _OtherExtents, enable_if_t<is_constructible_v<_Extents, _OtherExtents>, int> = 0>
constexpr explicit(!is_convertible_v<_OtherExtents, _Extents>)
mapping(const mapping<_OtherExtents>& _Other) noexcept
: _Myext{_Other.extents()} {};
: _Myext{_Other.extents()} {}

template <class _OtherExtents,
enable_if_t<_Extents::rank() <= 1 && is_constructible_v<_Extents, _OtherExtents>, int> = 0>
Expand Down Expand Up @@ -367,8 +379,8 @@ public:
return _Result;
}

template <class OtherExtents>
_NODISCARD friend constexpr bool operator==(const mapping& _Left, const mapping<OtherExtents>& _Right) noexcept {
template <class _OtherExtents>
_NODISCARD_FRIEND constexpr bool operator==(const mapping& _Left, const mapping<_OtherExtents>& _Right) noexcept {
return _Left.extents() == _Right.extents();
}

Expand All @@ -394,15 +406,21 @@ 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;

constexpr mapping(const _Extents& e) noexcept : _Myext(e){};
constexpr mapping(const _Extents& _Ext) noexcept : _Myext(_Ext) {}
Comment thread
JMazurkiewicz marked this conversation as resolved.

template <class _OtherExtents, enable_if_t<is_constructible_v<_Extents, _OtherExtents>, int> = 0>
constexpr explicit(!is_convertible_v<_OtherExtents, _Extents>)
mapping(const mapping<_OtherExtents>& _Other) noexcept
: _Myext{_Other.extents()} {};
: _Myext{_Other.extents()} {}

template <class _OtherExtents,
enable_if_t<_Extents::rank() <= 1 && is_constructible_v<_Extents, _OtherExtents>, int> = 0>
Expand Down Expand Up @@ -472,8 +490,8 @@ public:
return _Result;
}

template <class OtherExtents>
_NODISCARD friend constexpr bool operator==(const mapping& _Left, const mapping<OtherExtents>& _Right) noexcept {
template <class _OtherExtents>
_NODISCARD_FRIEND constexpr bool operator==(const mapping& _Left, const mapping<_OtherExtents>& _Right) noexcept {
return _Left.extents() == _Right.extents();
}

Expand Down Expand Up @@ -501,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;

Expand Down Expand Up @@ -618,7 +642,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;
}
Expand Down Expand Up @@ -705,6 +729,18 @@ 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(
_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<ElementType, typename AccessorPolicy::element_type> must be true (N4928 "
Comment thread
JMazurkiewicz marked this conversation as resolved.
"[mdspan.mdspan.overview]/2.3).");

_NODISCARD static constexpr rank_type rank() noexcept {
return _Extents::rank();
}
Expand All @@ -713,8 +749,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 <class _Mapping = mapping_type,
Expand All @@ -723,8 +759,8 @@ public:
int> = 0>
constexpr mdspan() {}

constexpr mdspan(const mdspan& rhs) = default;
constexpr mdspan(mdspan&& rhs) = default;
constexpr mdspan(const mdspan&) = default;
constexpr mdspan(mdspan&&) = default;

template <class _Mapping = mapping_type,
enable_if_t<(rank() == 0 || rank_dynamic() == 0)
Expand Down Expand Up @@ -780,12 +816,16 @@ public:
|| !is_convertible_v<const _OtherAccessor&, accessor_type>)
mdspan(const mdspan<_OtherElementType, _OtherExtents, _OtherLayoutPolicy, _OtherAccessor>& _Other)
: _Ptr{_Other._Ptr}, _Map{_Other._Map}, _Acc{_Other._Acc} {
static_assert(is_constructible_v<data_handle_type, const typename _OtherAccessor ::data_handle_type&>);
static_assert(is_constructible_v<extents_type, _OtherExtents>);
static_assert(is_constructible_v<data_handle_type, const typename _OtherAccessor::data_handle_type&>,
"Expression is_constructible_v<data_handle_type, const typename OtherAccessor::data_handle_type&> must "
"be true (N4928 [mdspan.mdspan.cons]/20.1).");
static_assert(is_constructible_v<extents_type, _OtherExtents>,
"Expression is_constructible_v<extents_type, OtherExtents> must 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 <class... _OtherIndexTypes,
Expand Down Expand Up @@ -823,9 +863,9 @@ public:
return _Acc;
}

_NODISCARD constexpr index_type extent(const rank_type r) const noexcept {
_NODISCARD constexpr index_type extent(const rank_type _Rank) const noexcept {
const auto& _Ext = _Map.extents();
return _Ext.extent(r);
return _Ext.extent(_Rank);
}

_NODISCARD constexpr size_type size() const noexcept {
Expand Down
31 changes: 18 additions & 13 deletions tests/std/tests/P0009R18_mdspan/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
};
Expand Down Expand Up @@ -180,13 +180,13 @@ void extent_tests_copy_ctor_other() {
static_assert(!is_constructible_v<extents<size_t, 2, 3>, extents<size_t, 2>>);
static_assert(!is_constructible_v<extents<size_t, 2, 3>, extents<size_t, 3, 2>>);

// 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<size_t, 2, 3>, extents<size_t, 2, dynamic_extent>>);
constexpr extents<size_t, 2, 3> ex0{extents<size_t, 2, dynamic_extent>{}};
(void) ex0;
static_assert(!is_convertible_v<extents<size_t, 2, dynamic_extent>, extents<size_t, 2, 3>>);

// Dynamic extents are constuctible and convertible from static extents.
// Dynamic extents are constructible and convertible from static extents.
static_assert(is_constructible_v<extents<size_t, 2, dynamic_extent>, extents<size_t, 2, 3>>);
extents<size_t, 2, dynamic_extent>{extents<size_t, 2, 3>{}};
static_assert(is_convertible_v<extents<size_t, 2, 3>, extents<size_t, 2, dynamic_extent>>);
Expand Down Expand Up @@ -776,8 +776,8 @@ void layout_stride_tests_ctor_other_extents() {
constexpr extents<size_t, 2, dynamic_extent> e2{3};
constexpr array<size_t, 2> s{3, 1};

constexpr layout_stride::mapping<decltype(e1)> m1(e1, s);
constexpr layout_stride::mapping<decltype(e2)> m2(m1);
constexpr layout_stride::mapping<remove_const_t<decltype(e1)>> m1(e1, s);
constexpr layout_stride::mapping<remove_const_t<decltype(e2)>> m2(m1);

static_assert(m2.extents() == e1);
static_assert(m2.strides() == s);
Expand Down Expand Up @@ -897,6 +897,7 @@ namespace Pathological {
};

struct Accessor {
using element_type = int;
using data_handle_type = int*;
using reference = int&;
Accessor(int) {}
Expand Down Expand Up @@ -927,10 +928,11 @@ void mdspan_tests_ctor_sizes() {
static_assert((mds1.extents() == extents<size_t, 2, 3>{}));
static_assert(mds1.is_exhaustive());

static_assert(!is_constructible_v<mdspan<int, Pathological::Extents, Pathological::Layout>, 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<mdspan<int, Pathological::Extents, Pathological::Layout>, 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<mdspan<int, Pathological::Extents, Pathological::Layout>, int*,
// int>); // Pathological::Extents not constructible from int

Expand All @@ -948,11 +950,13 @@ void mdspan_tests_ctor_array() {
static_assert(mds1.data_handle() == arr);
static_assert(mds1.extents() == extents<size_t, 2, 3>{});

static_assert(!is_constructible_v<mdspan<int, Pathological::Extents, Pathological::Layout>, int*,
array<Pathological::Empty, 1>>); // 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<mdspan<int, Pathological::Extents, Pathological::Layout>, int*,
// array<Pathological::Empty, 1>>); // Empty not convertible to size_type

static_assert(!is_constructible_v<mdspan<int, Pathological::Extents, Pathological::Layout>, int*,
array<int, 1>>); // 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<mdspan<int, Pathological::Extents, Pathological::Layout>, int*,
// array<int, 1>>); // Pathological::Extents not constructible from int

static_assert(!is_constructible_v<mdspan<int, extents<size_t, dynamic_extent>, Pathological::Layout>, int*,
array<int, 1>>); // Pathological::Layout not constructible from extents<size_t, dynamic_extent>
Expand Down Expand Up @@ -994,6 +998,7 @@ void mdspan_tests_ctor_mapping() {

template <class Type>
struct stateful_accessor {
using element_type = Type;
using data_handle_type = Type*;
using reference = Type&;

Expand Down