Skip to content

Commit

Permalink
* Add mdspan deduction guides for the array extents constructor and
Browse files Browse the repository at this point in the history
  mapping/accessor constructors. Now all constructors have deduction guides.
* Improve the `mdspan` deduction guide for the constructor that takes a
  parameter pack of dynamic indices.
* Change `layout_stride::mapping`'s constructor to take a `dextents` for the
  strides instead of an `array`. This is both cleaner, and was needed to make
  CTAD work.
* Add a `layout` type alias to all layout mappings, which is needed by
  `mdspan`'s deduction guides for its layout mapping constructors.
* Move all `mdspan` initialization and CTAD tests into `test_mdspan_ctors`.
* Add `layout_(left|right)` and `layout_stride` initialization and CTAD tests to
  `test_layout_ctors` and `test_layout_stride` respectively.
* Add a test for `mdspan` construction from a C-array.
* Add a test for `mdspan` construction from an extents `array` of type
  convertible to, but not exactly, `size_t`.
  • Loading branch information
brycelelbach committed Jun 9, 2021
1 parent 396d733 commit 2141453
Show file tree
Hide file tree
Showing 13 changed files with 476 additions and 180 deletions.
25 changes: 21 additions & 4 deletions include/experimental/__p0009_bits/basic_mdspan.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,10 +330,27 @@ class basic_mdspan<

};

#if _MDSPAN_USE_DEDUCTION_GUIDES
template <class ElementType, class... Integrals>
explicit basic_mdspan(ElementType*, Integrals...)
-> basic_mdspan<ElementType, extents<detail::__make_dynamic_extent<Integrals>()...>>;
#if _MDSPAN_USE_CLASS_TEMPLATE_ARGUMENT_DEDUCTION
MDSPAN_TEMPLATE_REQUIRES(
class ElementType, class... SizeTypes,
/* requires */ _MDSPAN_FOLD_AND(_MDSPAN_TRAIT(is_integral, SizeTypes) /* && ... */)
)
explicit basic_mdspan(ElementType*, SizeTypes...)
-> basic_mdspan<ElementType, dextents<sizeof...(SizeTypes)>>;

template <class ElementType, class SizeType, size_t N>
explicit basic_mdspan(ElementType*, const array<SizeType, N>&)
-> basic_mdspan<ElementType, dextents<N>>;

// TODO @proposal-bug Layout mappings in the proposal are not required to have `extents_type`.
template <class ElementType, class MappingType>
basic_mdspan(ElementType*, const MappingType&)
-> basic_mdspan<ElementType, typename MappingType::extents_type, typename MappingType::layout>;

// TODO @proposal-bug Layout mappings in the proposal are not required to have `extents_type`.
template <class ElementType, class MappingType, class AccessorType>
basic_mdspan(ElementType*, const MappingType&, const AccessorType&)
-> basic_mdspan<ElementType, typename MappingType::extents_type, typename MappingType::layout, AccessorType>;
#endif

template <class T, size_t... Exts>
Expand Down
30 changes: 28 additions & 2 deletions include/experimental/__p0009_bits/layout_left.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,26 @@ struct layout_left_idx_conditional {
struct layout_left {
template <class Extents>
class mapping {
public:
static_assert(detail::__depend_on_instantiation_v<Extents, false>, "layout_left::mapping instantiated with invalid extents type.");

#if _MDSPAN_USE_CLASS_TEMPLATE_ARGUMENT_DEDUCTION
// GCC currently rejects deduction guides for member template classes at
// class scope, so we can't write the deduction guide we need for mappings.
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100983
//
// For this layout, we shouldn't need a deduction guide; the automatic ones
// should work. But, in our implementation we've chosen to leave the primary
// template empty and put the implementation in the specialization below.
// Unfortunately, it seems that the primary template is the one used for
// deduction.
//
// But, by cleverly adding a constructor declaration to the primary that
// serves as a pseudo deduction guide, we can get the automatic deduction
// guide to do the right thing.
MDSPAN_INLINE_FUNCTION
constexpr mapping(Extents const&) noexcept = delete;
#endif
};

template <size_t... Exts>
Expand Down Expand Up @@ -97,6 +116,14 @@ struct layout_left {

using base_t::base_t;

// TODO @proposal-bug This isn't a requirement of layouts in the proposal,
// but we need it for `mdspan`'s deduction guides for its mapping
// constructors.
using layout = layout_left;

// TODO @proposal-bug This isn't a requirement in the proposal.
using typename base_t::extents_type;

// TODO noexcept specification
MDSPAN_TEMPLATE_REQUIRES(
class OtherExtents,
Expand All @@ -109,7 +136,6 @@ struct layout_left {
: base_t(other.extents())
{ }


// TODO noexcept specification
MDSPAN_TEMPLATE_REQUIRES(
class OtherExtents,
Expand Down Expand Up @@ -142,7 +168,7 @@ struct layout_left {
}

};
};
};

} // end namespace experimental
} // end namespace std
27 changes: 27 additions & 0 deletions include/experimental/__p0009_bits/layout_right.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,26 @@ struct layout_right_idx_conditional {
struct layout_right {
template <class Extents>
class mapping {
public:
static_assert(detail::__depend_on_instantiation_v<Extents, false>, "layout_right::mapping instantiated with invalid extents type.");

#if _MDSPAN_USE_CLASS_TEMPLATE_ARGUMENT_DEDUCTION
// GCC currently rejects deduction guides for member template classes at
// class scope, so we can't write the deduction guide we need for mappings.
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100983
//
// For this layout, we shouldn't need a deduction guide; the automatic ones
// should work. But, in our implementation we've chosen to leave the primary
// template empty and put the implementation in the specialization below.
// Unfortunately, it seems that the primary template is the one used for
// deduction.
//
// But, by cleverly adding a constructor declaration to the primary that
// serves as a pseudo deduction guide, we can get the automatic deduction
// guide to do the right thing.
MDSPAN_INLINE_FUNCTION
constexpr mapping(Extents const&) noexcept = delete;
#endif
};

template <size_t... Exts>
Expand Down Expand Up @@ -97,6 +116,14 @@ struct layout_right {

using base_t::base_t;

// TODO @proposal-bug This isn't a requirement of layouts in the proposal,
// but we need it for `mdspan`'s deduction guides for its mapping
// constructors.
using layout = layout_right;

// TODO @proposal-bug This isn't a requirement in the proposal.
using typename base_t::extents_type;

// TODO noexcept specification
MDSPAN_TEMPLATE_REQUIRES(
class OtherExtents,
Expand Down
29 changes: 24 additions & 5 deletions include/experimental/__p0009_bits/layout_stride.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,24 @@ struct layout_stride {
template <class Extents>
class mapping {
static_assert(detail::__depend_on_instantiation_v<Extents, false>, "layout_stride::mapping instantiated with invalid extents type.");

#if _MDSPAN_USE_CLASS_TEMPLATE_ARGUMENT_DEDUCTION
// GCC currently rejects deduction guides for member template classes at
// class scope, so we can't write the deduction guide we need for mappings.
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100983
//
// (Un)fortunately, with an oh-so-clever constructor declaration in the
// primary that serves as a pseudo deduction guide, we can get the
// automatic deduction guide to do the right thing.
MDSPAN_INLINE_FUNCTION
constexpr mapping(Extents const&, dextents<Extents::rank()> const&) noexcept = delete;
#endif
};

template <size_t... Exts>
class mapping<
std::experimental::extents<Exts...>
>
>
#if !defined(_MDSPAN_USE_ATTRIBUTE_NO_UNIQUE_ADDRESS)
: private detail::__no_unique_address_emulation<
detail::__compressed_pair<
Expand All @@ -85,6 +97,11 @@ struct layout_stride {

using extents_type = experimental::extents<Exts...>;

// TODO @proposal-bug This isn't a requirement of layouts in the proposal,
// but we need it for `mdspan`'s deduction guides for its mapping
// constructors.
using layout = layout_stride;

private:

using idx_seq = make_index_sequence<sizeof...(Exts)>;
Expand Down Expand Up @@ -217,19 +234,21 @@ struct layout_stride {
mapping& operator=(mapping&&) noexcept = default;
MDSPAN_INLINE_FUNCTION_DEFAULTED ~mapping() noexcept = default;

// TODO @proposal-bug layout stride needs this constructor
// TODO @proposal-bug In the proposal, the constructor doesn't take a
// `dextents`, and doesn't accept any `array` with elements convertible to
// `size_t`.
MDSPAN_INLINE_FUNCTION
constexpr
mapping(
std::experimental::extents<Exts...> const& e,
array<size_t, __strides_storage_t::rank()> const& strides
std::experimental::dextents<__strides_storage_t::rank()> const& strides
) noexcept
#if defined(_MDSPAN_USE_ATTRIBUTE_NO_UNIQUE_ADDRESS)
: __members{
#else
: __base_t(__base_t{__member_pair_t(
#endif
e, __strides_storage_t(strides)
e, __strides_storage_t{strides}
#if defined(_MDSPAN_USE_ATTRIBUTE_NO_UNIQUE_ADDRESS)
}
#else
Expand All @@ -243,7 +262,7 @@ struct layout_stride {
MDSPAN_INLINE_FUNCTION constexpr extents_type extents() const noexcept {
#if defined(_MDSPAN_USE_ATTRIBUTE_NO_UNIQUE_ADDRESS)
return __members.__first();
#else
#else
return this->__base_t::__ref().__first();
#endif
};
Expand Down
2 changes: 1 addition & 1 deletion include/experimental/__p0009_bits/macros.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@
#else
# define _MDSPAN_DEDUCE_RETURN_TYPE_SINGLE_LINE(SIGNATURE, BODY) \
auto MDSPAN_PP_REMOVE_PARENS(SIGNATURE) \
-> typename std::remove_cv<typename std::remove_reference<decltype(BODY)>::type>::type \
-> std::remove_cv_t<std::remove_reference_t<decltype(BODY)>> \
{ return MDSPAN_PP_REMOVE_PARENS(BODY); }
# define _MDSPAN_DEDUCE_DECLTYPE_AUTO_RETURN_TYPE_SINGLE_LINE(SIGNATURE, BODY) \
auto MDSPAN_PP_REMOVE_PARENS(SIGNATURE) \
Expand Down
4 changes: 2 additions & 2 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ add_subdirectory(${CMAKE_CURRENT_BINARY_DIR}/googletest-src

mdspan_add_test(test_extents)
mdspan_add_test(test_contiguous_layouts)
mdspan_add_test(test_ctors)
mdspan_add_test(test_mdspan_ctors)
mdspan_add_test(test_mdspan_conversion)
mdspan_add_test(test_layout_ctors)
mdspan_add_test(test_layout_stride)
mdspan_add_test(test_element_access)
mdspan_add_test(test_conversion)

64 changes: 0 additions & 64 deletions tests/test_contiguous_layouts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,67 +166,3 @@ TYPED_TEST(TestLayout3D, mapping_works) {
TYPED_TEST(TestLayout3D, required_span_size_works) {
ASSERT_EQ(this->map.required_span_size(), 3*4*5);
}

TEST(TestLayoutLeftListInitialization, test_layout_left_list_initialization) {
double* data = nullptr;
stdex::basic_mdspan<double, stdex::extents<dyn, dyn>, stdex::layout_left> m(data, {{16, 32}});
ASSERT_EQ(m.rank(), 2);
ASSERT_EQ(m.rank_dynamic(), 2);
ASSERT_EQ(m.stride(0), 1);
ASSERT_EQ(m.stride(1), 16);
ASSERT_EQ(m.extent(0), 16);
ASSERT_EQ(m.extent(1), 32);
ASSERT_TRUE(m.is_contiguous());
}

// GCC 10 ICEs if I stick this in the body of the below test.
template <typename M>
void test_layout_left_ctad_gcc_10_workaround(M m) {
ASSERT_EQ(m.rank(), 2);
ASSERT_EQ(m.rank_dynamic(), 2);
ASSERT_EQ(m.stride(0), 1);
ASSERT_EQ(m.stride(1), 32);
ASSERT_EQ(m.extent(0), 16);
ASSERT_EQ(m.extent(1), 32);
}

// TODO: this fails to compile on GCC 9.2 and clang 12 and likely others
/*
TEST(TestLayoutLeftCTAD, test_layout_left_ctad) {
double* data = nullptr;
stdex::basic_mdspan m(data, stdex::layout_left::mapping{stdex::extents{16, 32}});
test_layout_left_ctad_gcc_10_workaround(m);
}
*/

TEST(TestLayoutRightListInitialization, test_layout_right_list_initialization) {
double* data = nullptr;
stdex::basic_mdspan<double, stdex::extents<dyn, dyn>, stdex::layout_right> m(data, {{16, 32}});
ASSERT_EQ(m.rank(), 2);
ASSERT_EQ(m.rank_dynamic(), 2);
ASSERT_EQ(m.stride(0), 32);
ASSERT_EQ(m.stride(1), 1);
ASSERT_EQ(m.extent(0), 16);
ASSERT_EQ(m.extent(1), 32);
ASSERT_TRUE(m.is_contiguous());
}

// GCC 10 ICEs if I stick this in the body of the below test.
template <typename M>
void test_layout_right_ctad_gcc_10_workaround(M m) {
ASSERT_EQ(m.rank(), 2);
ASSERT_EQ(m.rank_dynamic(), 2);
ASSERT_EQ(m.stride(0), 1);
ASSERT_EQ(m.stride(1), 32);
ASSERT_EQ(m.extent(0), 16);
ASSERT_EQ(m.extent(1), 32);
}

// This fails on GCC 9.2 and clang 12 and likely others
/*
TEST(TestLayoutRightCTAD, test_layout_right_ctad) {
double* data = nullptr;
stdex::basic_mdspan m(data, stdex::layout_right::mapping{stdex::extents{16, 32}});
test_layout_right_ctad_gcc_10_workaround(m);
}
*/
58 changes: 0 additions & 58 deletions tests/test_ctors.cpp

This file was deleted.

9 changes: 9 additions & 0 deletions tests/test_extents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,15 @@ TYPED_TEST(TestExtentsCompatCtors, compatible_assign_2) {
EXPECT_EQ(this->exts1, this->exts2);
}

TEST(TestExtentsCtorStdArrayConvertibleToSizeT, test_extents_ctor_std_array_convertible_to_size_t) {
std::array<int, 2> i{2, 2};
stdex::dextents<2> e{i};
ASSERT_EQ(e.rank(), 2);
ASSERT_EQ(e.rank_dynamic(), 2);
ASSERT_EQ(e.extent(0), 2);
ASSERT_EQ(e.extent(1), 2);
}

#if _MDSPAN_USE_CLASS_TEMPLATE_ARGUMENT_DEDUCTION
TEST(TestExtentsCTADPack, test_extents_ctad_pack) {
stdex::extents m0;
Expand Down
Loading

0 comments on commit 2141453

Please sign in to comment.