Fix canonical_ice & 1 strided_slice

submdspan_canonicalize_slices now works for one strided_slice.
Fix canonical_ice (wrong order in subtraction; wording is fine).

Author: mhoemmen

include/experimental/__p2630_bits/submdspan_extents.hpp

@@ -448,7 +448,7 @@ constexpr auto canonical_ice(S s) {
 
 template<class IndexType, class X, class Y>
 constexpr auto subtract_ice(X x, Y y) {
-  return canonical_ice<IndexType>(x) - canonical_ice<IndexType>(y);
+  return canonical_ice<IndexType>(y) - canonical_ice<IndexType>(x);
 }
 
 template<class T>
@@ -560,10 +560,19 @@ template<size_t k, class S_k, class IndexType, size_t... Exts>
     // well-formed if it didn't fall into one of the above cases
     // and if it can't be destructured into two elements.
 
-    // We can't use s_k here, because it's not a constant expression.
-    auto [s_k0, s_k1] = S_k{};
-    using S_k0 = decltype(s_k0);
-    using S_k1 = decltype(s_k1);
+    // We can't use s_k on the right-hand side here, because it's not a constant expression.
+    // We can't use S_k{} here either, because that presumes that it's default constructible.
+    // We can only use std::declval<S_k>() in an unevaluated context.
+    auto get_first = [] (S_k s_k) {
+      auto [s_k0, _] = s_k;
+      return s_k0;
+    };
+    auto get_second = [] (S_k s_k) {
+      auto [_, s_k1] = s_k;
+      return s_k1;
+    };
+    using S_k0 = decltype(get_first(std::declval<S_k>()));
+    using S_k1 = decltype(get_second(std::declval<S_k>()));
     if constexpr (__mdspan_integral_constant_like<S_k0>) {
       if constexpr (de_ice(S_k0{}) < 0) {
         return check_static_bounds_result::out_of_bounds; // 14.4.1
@@ -703,6 +712,10 @@ submdspan_canonicalize_one_slice(const extents<IndexType, Extents...>& exts, Sli
     using S_k1 = decltype(s_k1);
     static_assert(std::is_convertible_v<S_k0, IndexType>);
     static_assert(std::is_convertible_v<S_k1, IndexType>);
+
+    static_assert(std::is_same_v<decltype(canonical_ice<IndexType>(s_k0)), IndexType>);
+    static_assert(std::is_same_v<decltype(subtract_ice<IndexType>(s_k0, s_k1)), IndexType>);
+
     return strided_slice{
       .offset = canonical_ice<IndexType>(s_k0),
       .extent = subtract_ice<IndexType>(s_k0, s_k1),

tests/test_canonicalize_slices.cpp

@@ -22,6 +22,61 @@
 #  error "This file requires MDSPAN_ENABLE_P3663=ON"
 #endif
 
+namespace my_test {
+
+template<class First, class Second>
+struct my_aggregate_pair {
+  First first;
+  Second second;
+};
+
+// Not an aggregate, to force use of the tuple protocol.
+template<class First, class Second>
+class my_nonaggregate_pair {
+public:
+  constexpr my_nonaggregate_pair(First first, Second second)
+    : first_(first), second_(second)
+  {}
+
+  template<std::size_t Index, class Self>
+  constexpr decltype(auto) get(this Self&& self) {
+    if constexpr (Index == 0) {
+      return self.first_;
+    }
+    else if constexpr (Index == 1) {
+      return self.second_;
+    }
+    else {
+      static_assert(false, "Invalid index");
+    }
+  }
+
+private:
+  First first_;
+  Second second_;
+};
+
+} // namespace my_test
+
+template<class First, class Second>
+struct std::tuple_size<my_test::my_nonaggregate_pair<First, Second>>
+  : std::integral_constant<std::size_t, 2> {};
+
+template<std::size_t Index, class First, class Second>
+struct std::tuple_element<Index, my_test::my_nonaggregate_pair<First, Second>> {
+  static_assert(false, "Invalid index");
+};
+
+template<class First, class Second>
+struct std::tuple_element<0, my_test::my_nonaggregate_pair<First, Second>> {
+  using type = First;
+};
+
+template<class First, class Second>
+struct std::tuple_element<1, my_test::my_nonaggregate_pair<First, Second>> {
+  using type = Second;
+};
+
 namespace {
 
 template<class T>
@@ -44,6 +99,14 @@ constexpr bool slice_equal(const Left&, Kokkos::full_extent_t) {
   return std::is_convertible_v<Left, Kokkos::full_extent_t>;  
 }
 
+template<class OffsetType, class ExtentType, class StrideType>
+constexpr bool slice_equal(
+  const Kokkos::strided_slice<OffsetType, ExtentType, StrideType>& left,
+  const Kokkos::strided_slice<OffsetType, ExtentType, StrideType>& right)
+{
+  return left.offset == right.offset && left.extent == right.extent && left.stride == right.stride;
+}
+
 template<class ExpectedResult, class InputExtents, class... Slices>
 void
 test_canonicalize_slices(
@@ -92,4 +155,26 @@ TEST(CanonicalizeSlices, Rank1_integer_static) {
   test_canonicalize_slices(expected_slices, exts, slice0);
 }
 
+TEST(CanonicalizeSlices, Rank1_aggregate_pair) {
+  constexpr auto slice0 = my_test::my_aggregate_pair<int, int>{7, 11};
+  constexpr auto expected_slices = std::tuple{Kokkos::strided_slice{
+    .offset = size_t(7u),
+    .extent = (size_t(11u) - size_t(7u)),
+    .stride = std::cw<size_t(1u)>
+  }};
+  constexpr auto exts = Kokkos::extents<size_t, 13>{};
+  test_canonicalize_slices(expected_slices, exts, slice0);
+}
+
+TEST(CanonicalizeSlices, Rank1_nonaggregate_pair) {
+  constexpr auto slice0 = my_test::my_nonaggregate_pair<int, int>(7, 11);
+  constexpr auto expected_slices = std::tuple{Kokkos::strided_slice{
+    .offset = size_t(7u),
+    .extent = (size_t(11u) - size_t(7u)),
+    .stride = std::cw<size_t(1u)>
+  }};
+  constexpr auto exts = Kokkos::extents<size_t, 13>{};
+  test_canonicalize_slices(expected_slices, exts, slice0);
+}
+
 } // namespace (anonymous)