Skip to content
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

<mdspan>: Add new debug checks to layout mappings #3702

Merged
merged 2 commits into from
May 15, 2023
Merged

<mdspan>: Add new debug checks to layout mappings #3702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da9aad9
New checks!
JMazurkiewicz May 14, 2023
dbb1a61
New checks detected a bug in the tests!
JMazurkiewicz May 14, 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
23 changes: 23 additions & 0 deletions stl/inc/mdspan
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,17 @@ public:
return true;
}
}

template <class... _IndexTypes, size_t... _Seq>
_NODISCARD constexpr bool _Contains_multidimensional_index(
index_sequence<_Seq...>, _IndexTypes... _Indices) const noexcept {
_STL_INTERNAL_STATIC_ASSERT((same_as<_IndexTypes, index_type> && ...));
if constexpr (unsigned_integral<index_type>) {
return ((_Indices < extent(_Seq)) && ...);
} else {
return ((0 <= _Indices && _Indices < extent(_Seq)) && ...);
}
}
};

template <class>
Expand Down Expand Up @@ -379,6 +390,10 @@ public:
requires (sizeof...(_IndexTypes) == extents_type::rank()) && (is_convertible_v<_IndexTypes, index_type> && ...)
&& (is_nothrow_constructible_v<index_type, _IndexTypes> && ...)
_NODISCARD constexpr index_type operator()(_IndexTypes... _Indices) const noexcept {
_STL_VERIFY(_Exts._Contains_multidimensional_index(
make_index_sequence<extents_type::rank()>{}, static_cast<index_type>(_Indices)...),
"Value of extents_type::index-cast(i) must be a multidimensional index in extents_ (N4950 "
"[mdspan.layout.left.obs]/3).");
return _Index_impl(make_index_sequence<extents_type::rank()>{}, static_cast<index_type>(_Indices)...);
}

Expand Down Expand Up @@ -512,6 +527,10 @@ public:
requires (sizeof...(_IndexTypes) == extents_type::rank()) && (is_convertible_v<_IndexTypes, index_type> && ...)
&& (is_nothrow_constructible_v<index_type, _IndexTypes> && ...)
_NODISCARD constexpr index_type operator()(_IndexTypes... _Indices) const noexcept {
_STL_VERIFY(_Exts._Contains_multidimensional_index(
make_index_sequence<extents_type::rank()>{}, static_cast<index_type>(_Indices)...),
"Value of extents_type::index-cast(i) must be a multidimensional index in extents_ (N4950 "
"[mdspan.layout.right.obs]/3).");
return _Index_impl(make_index_sequence<extents_type::rank()>{}, static_cast<index_type>(_Indices)...);
}

Expand Down Expand Up @@ -698,6 +717,10 @@ public:
requires (sizeof...(_IndexTypes) == extents_type::rank()) && (is_convertible_v<_IndexTypes, index_type> && ...)
&& (is_nothrow_constructible_v<index_type, _IndexTypes> && ...)
_NODISCARD constexpr index_type operator()(_IndexTypes... _Indices) const noexcept {
_STL_VERIFY(_Exts._Contains_multidimensional_index(
make_index_sequence<extents_type::rank()>{}, static_cast<index_type>(_Indices)...),
"Value of extents_type::index-cast(i) must be a multidimensional index in extents_ (N4950 "
"[mdspan.layout.stride.obs]/3).");
return _Index_impl(make_index_sequence<extents_type::rank()>{}, static_cast<index_type>(_Indices)...);
}

Expand Down
2 changes: 1 addition & 1 deletion tests/std/tests/P0009R18_mdspan_layout_left/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ constexpr void check_correctness() {
assert(tensor(2, 0, 0) == 2);
assert(tensor(1, 1, 1) == 10);
assert(tensor(0, 0, 3) == 18);
assert(tensor(2, 2, 2) == 20);
assert(tensor(2, 1, 2) == 17);
assert(tensor(2, 1, 3) == 23);
}

Expand Down
7 changes: 7 additions & 0 deletions tests/std/tests/P0009R18_mdspan_layout_left_death/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ void test_construction_from_other_stride_mapping_2() {
layout_left::mapping<dextents<unsigned char, 1>> m2{m1};
}

void test_call_operator() {
layout_left::mapping<extents<int, 3, 4, 5>> m;
// Value of extents_type::index-cast(i) must be a multidimensional index in extents_
(void) m(2, 3, 5);
}

void test_stride_function() {
layout_left::mapping<extents<int, 3>> m;
// Value of i must be less than extents_type::rank()
Expand All @@ -52,6 +58,7 @@ int main(int argc, char* argv[]) {
test_construction_from_other_right_mapping,
test_construction_from_other_stride_mapping_1,
test_construction_from_other_stride_mapping_2,
test_call_operator,
test_stride_function,
});
return exec.run(argc, argv);
Expand Down
8 changes: 5 additions & 3 deletions tests/std/tests/P0009R18_mdspan_layout_right/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,15 @@ constexpr void check_members(const extents<IndexType, Extents...>& ext, index_se
using Ext2 = extents<OtherIndexType, Extents...>;
using Mapping2 = layout_right::mapping<Ext2>;

#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
#ifndef __clang__ // FIXME, Clang suddenly cannot digest this
{ // Check construction from other layout_right::mapping
Mapping m1{ext};
Mapping2 m2{m1};
assert(m1 == m2);
static_assert(is_nothrow_constructible_v<Mapping2, Mapping>);
// Other tests are defined in 'check_construction_from_other_right_mapping' function
}
#endif
JMazurkiewicz marked this conversation as resolved.
Show resolved Hide resolved

{ // Check construction from layout_left::mapping
using LeftMapping = layout_left::mapping<Ext>;
Expand All @@ -82,7 +82,10 @@ constexpr void check_members(const extents<IndexType, Extents...>& ext, index_se
if constexpr (Ext::rank() > 0) {
strides.back() = 1;
for (size_t i = Ext::rank() - 1; i-- > 0;) {
#pragma warning(push)
#pragma warning(disable : 28020) // TRANSITION, DevCom-923103
strides[i] = static_cast<IndexType>(strides[i + 1] * ext.extent(i + 1));
#pragma warning(pop)
}
}

Expand Down Expand Up @@ -140,7 +143,6 @@ constexpr void check_members(const extents<IndexType, Extents...>& ext, index_se
assert(!(m != m));
// Other tests are defined in 'check_comparisons' function
}
#pragma warning(pop) // TRANSITION, "/analyze:only" BUG?
}

constexpr void check_construction_from_other_right_mapping() {
Expand Down
7 changes: 7 additions & 0 deletions tests/std/tests/P0009R18_mdspan_layout_right_death/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ void test_construction_from_other_stride_mapping_2() {
layout_right::mapping<dextents<unsigned char, 1>> m2{m1};
}

void test_call_operator() {
layout_right::mapping<extents<int, 5, 4, 3>> m;
// Value of extents_type::index-cast(i) must be a multidimensional index in extents_
(void) m(4, 3, 3);
}

void test_stride_function() {
layout_right::mapping<extents<int, 3>> m;
// Value of i must be less than extents_type::rank()
Expand All @@ -52,6 +58,7 @@ int main(int argc, char* argv[]) {
test_construction_from_other_left_mapping,
test_construction_from_other_stride_mapping_1,
test_construction_from_other_stride_mapping_2,
test_call_operator,
test_stride_function,
});
return exec.run(argc, argv);
Expand Down
7 changes: 7 additions & 0 deletions tests/std/tests/P0009R18_mdspan_layout_stride_death/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,20 @@ void test_construction_from_strided_layout_mapping() {
[[maybe_unused]] layout_stride::mapping<dextents<unsigned char, 1>> m2{m1};
}

void test_call_operator() {
layout_stride::mapping<extents<unsigned, 5, 3, 4>> m;
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick for followup: We conventionally say unsigned int instead of unsigned.

// Value of extents_type::index-cast(i) must be a multidimensional index in extents_
(void) m(4, 3, 3);
}

int main(int argc, char* argv[]) {
std_testing::death_test_executive exec;
exec.add_death_tests({
// TRANSITION more tests
test_construction_from_extents_and_array,
test_construction_from_extents_and_span,
test_construction_from_strided_layout_mapping,
test_call_operator,
});
return exec.run(argc, argv);
}