Merge pull request #238 from youyu3/fix-201-stride-constraint

Add constraint for `layout_left|right|stride::stride()`, and add test
kokkos · Feb 24, 2023 · cf374bc · cf374bc
2 parents 458bd01 + 45a00ca
commit cf374bc
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 17 deletions.
diff --git a/README.md b/README.md
@@ -66,6 +66,7 @@ When not in C++23 mode the implementation deviates from the proposal as follows:
 - mdspan has a default constructor even in cases where it shouldn't (i.e. all static extents, and default constructible mapping/accessor)
 - the conditional explicit markup is missing, making certain constructors implicit
   - most notably you can implicitly convert from dynamic extent to static extent, which you can't in C++20 mode
+- there is a constraint on `layout_left::mapping::stride()`, `layout_right::mapping::stride()` and `layout_stride::mapping::stride()` that `extents_type::rank() > 0` is `true`, which is not implemented in C++17 or C++14.
 
 ### C++14
 - deduction guides don't exist

diff --git a/include/experimental/__p0009_bits/layout_left.hpp b/include/experimental/__p0009_bits/layout_left.hpp
@@ -177,7 +177,11 @@ class layout_left::mapping {
     MDSPAN_INLINE_FUNCTION constexpr bool is_strided() const noexcept { return true; }
 
     MDSPAN_INLINE_FUNCTION
-    constexpr index_type stride(rank_type i) const noexcept {
+    constexpr index_type stride(rank_type i) const noexcept
+#if MDSPAN_HAS_CXX_20
+      requires ( Extents::rank() > 0 )
+#endif
+    {
       index_type value = 1;
       for(rank_type r=0; r<i; r++) value*=__extents.extent(r);
       return value;

diff --git a/include/experimental/__p0009_bits/layout_right.hpp b/include/experimental/__p0009_bits/layout_right.hpp
@@ -179,7 +179,11 @@ class layout_right::mapping {
     MDSPAN_INLINE_FUNCTION constexpr bool is_strided() const noexcept { return true; }
 
     MDSPAN_INLINE_FUNCTION
-    constexpr index_type stride(rank_type i) const noexcept {
+    constexpr index_type stride(rank_type i) const noexcept
+#if MDSPAN_HAS_CXX_20
+      requires ( Extents::rank() > 0 )
+#endif
+    {
       index_type value = 1;
       for(rank_type r=extents_type::rank()-1; r>i; r--) value*=__extents.extent(r);
       return value;

diff --git a/include/experimental/__p0009_bits/layout_stride.hpp b/include/experimental/__p0009_bits/layout_stride.hpp
@@ -432,7 +432,11 @@ struct layout_stride {
 
 
     MDSPAN_INLINE_FUNCTION
-    constexpr index_type stride(rank_type r) const noexcept {
+    constexpr index_type stride(rank_type r) const noexcept
+#if MDSPAN_HAS_CXX_20
+      requires ( Extents::rank() > 0 )
+#endif
+    {
       return __strides_storage()[r];
     }
 

diff --git a/tests/test_exhaustive_layouts.cpp b/tests/test_exhaustive_layouts.cpp
@@ -391,9 +391,15 @@ TYPED_TEST(TestLayoutConversion, implicit_conversion) {
   #endif
     map1 = typename TestFixture::map_1_t(map2);
 
-  for(size_t r=0; r != this->exts1.rank(); ++r) {
-    ASSERT_EQ(map1.extents().extent(r), map2.extents().extent(r));
-    ASSERT_EQ(map1.stride(r), map2.stride(r));
+#if MDSPAN_HAS_CXX_20
+  if constexpr (TestFixture::exts_1_t::rank() > 0) {
+#endif
+    for(size_t r=0; r != this->exts1.rank(); ++r) {
+      ASSERT_EQ(map1.extents().extent(r), map2.extents().extent(r));
+      ASSERT_EQ(map1.stride(r), map2.stride(r));
+    }
+#if MDSPAN_HAS_CXX_20
   }
+#endif
 }
 
diff --git a/tests/test_layout_ctors.cpp b/tests/test_layout_ctors.cpp
@@ -241,3 +241,37 @@ TEST(TestLayoutRightCTAD, test_layout_right_ctad) {
   ASSERT_TRUE(m.is_exhaustive());
 }
 #endif
+
+#if MDSPAN_HAS_CXX_20
+template< class T, class RankType, class = void >
+struct is_stride_avail : std::false_type {};
+
+template< class T, class RankType >
+struct is_stride_avail< T
+                      , RankType
+                      , std::enable_if_t< std::is_same< decltype( std::declval<T>().stride( std::declval<RankType>() ) )
+                                                      , typename T::index_type
+                                                      >::value
+                                        >
+                      > : std::true_type {};
+
+TEST(TestLayoutLeftStrideConstraint, test_layout_left_stride_constraint) {
+  stdex::extents<int,16> ext1d{};
+  stdex::layout_left::mapping m1d{ext1d};
+  ASSERT_TRUE ((is_stride_avail< decltype(m1d), int >::value));
+
+  stdex::extents<int> ext0d{};
+  stdex::layout_left::mapping m0d{ext0d};
+  ASSERT_FALSE((is_stride_avail< decltype(m0d), int >::value));
+}
+
+TEST(TestLayoutRightStrideConstraint, test_layout_right_stride_constraint) {
+  stdex::extents<int,16> ext1d{};
+  stdex::layout_right::mapping m1d{ext1d};
+  ASSERT_TRUE ((is_stride_avail< decltype(m1d), int >::value));
+
+  stdex::extents<int> ext0d{};
+  stdex::layout_right::mapping m0d{ext0d};
+  ASSERT_FALSE((is_stride_avail< decltype(m0d), int >::value));
+}
+#endif
diff --git a/tests/test_layout_stride.cpp b/tests/test_layout_stride.cpp
@@ -205,3 +205,26 @@ TEST(TestLayoutStrideCTAD, test_ctad) {
 }
 #endif
 
+#if MDSPAN_HAS_CXX_20
+template< class T, class RankType, class = void >
+struct is_stride_avail : std::false_type {};
+
+template< class T, class RankType >
+struct is_stride_avail< T
+                      , RankType
+                      , std::enable_if_t< std::is_same< decltype( std::declval<T>().stride( std::declval<RankType>() ) )
+                                                      , typename T::index_type
+                                                      >::value
+                                        >
+                      > : std::true_type {};
+
+TEST(TestLayoutStrideStrideConstraint, test_layout_stride_stride_constraint) {
+  stdex::extents<int,16> ext1d{};
+  stdex::layout_stride::mapping m1d{ext1d, std::array<int,1>{1}};
+  ASSERT_TRUE ((is_stride_avail< decltype(m1d), int >::value));
+
+  stdex::extents<int> ext0d{};
+  stdex::layout_stride::mapping m0d{ext0d, std::array<int,0>{}};
+  ASSERT_FALSE((is_stride_avail< decltype(m0d), int >::value));
+}
+#endif
diff --git a/tests/test_mdarray_ctors.cpp b/tests/test_mdarray_ctors.cpp
@@ -141,18 +141,24 @@ void check_correctness(MDA& m, size_t rank, size_t rank_dynamic,
                        bool exhaustive) {
   ASSERT_EQ(m.rank(), rank);
   ASSERT_EQ(m.rank_dynamic(), rank_dynamic);
-  if(rank>0) {
-    ASSERT_EQ(m.extent(0), extent_0);
-    ASSERT_EQ(m.stride(0), stride_0);
-  }
-  if(rank>1) {
-    ASSERT_EQ(m.extent(1), extent_1);
-    ASSERT_EQ(m.stride(1), stride_1);
-  }
-  if(rank>2) {
-    ASSERT_EQ(m.extent(2), extent_2);
-    ASSERT_EQ(m.stride(2), stride_2);
+#if MDSPAN_HAS_CXX_20
+  if constexpr (MDA::extents_type::rank()>0) {
+#endif
+    if(rank>0) {
+      ASSERT_EQ(m.extent(0), extent_0);
+      ASSERT_EQ(m.stride(0), stride_0);
+    }
+    if(rank>1) {
+      ASSERT_EQ(m.extent(1), extent_1);
+      ASSERT_EQ(m.stride(1), stride_1);
+    }
+    if(rank>2) {
+      ASSERT_EQ(m.extent(2), extent_2);
+      ASSERT_EQ(m.stride(2), stride_2);
+    }
+#if MDSPAN_HAS_CXX_20
   }
+#endif
   if(ptr_matches)
     ASSERT_EQ(m.data(),ptr);
   else