From 89afaeafa1b5775367dfcefab1ef56cf04623473 Mon Sep 17 00:00:00 2001 From: Nicolas Morales Date: Mon, 2 Jun 2025 15:40:34 -0400 Subject: [PATCH 1/7] #409: add precondition checks in extents constructors --- include/experimental/__p0009_bits/extents.hpp | 25 ++++++++-- include/experimental/__p0009_bits/utility.hpp | 46 ++++++++++++++++++ tests/CMakeLists.txt | 2 +- tests/test_extents.cpp | 48 +++++++++++++++++++ 4 files changed, 115 insertions(+), 6 deletions(-) diff --git a/include/experimental/__p0009_bits/extents.hpp b/include/experimental/__p0009_bits/extents.hpp index ab10ba32..c8d8f6b7 100644 --- a/include/experimental/__p0009_bits/extents.hpp +++ b/include/experimental/__p0009_bits/extents.hpp @@ -439,7 +439,10 @@ template class extents { sizeof...(OtherIndexTypes) == m_rank_dynamic))) MDSPAN_INLINE_FUNCTION constexpr explicit extents(OtherIndexTypes... dynvals) noexcept - : m_vals(static_cast(dynvals)...) {} + : m_vals(static_cast(dynvals)...) { + MDSPAN_IMPL_PRECONDITION( + detail::all_values_are_nonnegative_and_representable(dynvals...)); + } MDSPAN_TEMPLATE_REQUIRES( class OtherIndexType, size_t N, @@ -452,7 +455,11 @@ template class extents { MDSPAN_INLINE_FUNCTION MDSPAN_CONDITIONAL_EXPLICIT(N != m_rank_dynamic) constexpr extents(const std::array &exts) noexcept - : m_vals(std::move(exts)) {} + : m_vals(std::move(exts)) { + MDSPAN_IMPL_PRECONDITION( + detail::range_is_nonnegative_and_representable( + std::begin(exts), std::end(exts))); + } #ifdef __cpp_lib_span MDSPAN_TEMPLATE_REQUIRES( @@ -464,7 +471,11 @@ template class extents { MDSPAN_INLINE_FUNCTION MDSPAN_CONDITIONAL_EXPLICIT(N != m_rank_dynamic) constexpr extents(const std::span &exts) noexcept - : m_vals(std::move(exts)) {} + : m_vals(std::move(exts)) { + MDSPAN_IMPL_PRECONDITION( + detail::range_is_nonnegative_and_representable( + std::begin(exts), std::end(exts))); + } #endif private: @@ -536,10 +547,14 @@ template class extents { ...) || (std::numeric_limits::max() < std::numeric_limits::max())) - constexpr extents(const extents &other) noexcept + constexpr extents( + const extents &other) noexcept : m_vals(impl_construct_vals_from_extents( std::integral_constant(), - std::integral_constant(), other)) {} + std::integral_constant(), other)) { + MDSPAN_IMPL_PRECONDITION( + detail::extent_is_representable(other)); + } // Comparison operator template diff --git a/include/experimental/__p0009_bits/utility.hpp b/include/experimental/__p0009_bits/utility.hpp index 2ded4363..c33a3a18 100644 --- a/include/experimental/__p0009_bits/utility.hpp +++ b/include/experimental/__p0009_bits/utility.hpp @@ -202,6 +202,52 @@ MDSPAN_INLINE_FUNCTION constexpr bool in_range(T t) noexcept { cmp_less_equal(t, std::numeric_limits::max()); } +template +MDSPAN_INLINE_FUNCTION constexpr bool is_nonnegative_and_representable(T t) noexcept { + if constexpr (std::is_signed_v) { + if (t < 0) + return false; + } + + return in_range(t); +} + +template +MDSPAN_INLINE_FUNCTION constexpr bool +all_values_are_representable(Values... values) noexcept { + return ( in_range( values ) && ... && true ); +} + +template +MDSPAN_INLINE_FUNCTION constexpr bool +all_values_are_nonnegative_and_representable(Values... values) noexcept { + return ( is_nonnegative_and_representable( values ) && ... && true ); +} + +template +MDSPAN_INLINE_FUNCTION constexpr bool +range_is_nonnegative_and_representable(ContiguousIterator begin, ContiguousIterator end) noexcept { + for ( auto it = begin; it != end; ++it ) + { + if ( !is_nonnegative_and_representable( *it ) ) + return false; + } + + return true; +} + +template +MDSPAN_INLINE_FUNCTION constexpr bool +extent_is_representable(const Extents &exts) noexcept { + for ( std::size_t r = 0; r < Extents::rank(); ++r ) + { + if ( !is_nonnegative_and_representable( exts.extent(r) ) ) + return false; + } + + return true; +} + template MDSPAN_INLINE_FUNCTION constexpr bool check_mul_result_is_nonnegative_and_representable(T a, T b) { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b038ebcc..97c7fdb9 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -78,7 +78,7 @@ mdspan_add_test(test_macros ENABLE_PRECONDITIONS) mdspan_add_test(test_layout_preconditions ENABLE_PRECONDITIONS) mdspan_add_test(test_dims) -mdspan_add_test(test_extents) +mdspan_add_test(test_extents ENABLE_PRECONDITIONS) mdspan_add_test(test_mdspan_at) mdspan_add_test(test_mdspan_ctors) mdspan_add_test(test_mdspan_swap) diff --git a/tests/test_extents.cpp b/tests/test_extents.cpp index 718fc4a4..6a0fe8dc 100644 --- a/tests/test_extents.cpp +++ b/tests/test_extents.cpp @@ -481,3 +481,51 @@ TEST(TestExtentsCTADStdArray, test_extents_ctad_std_array) { } */ #endif + +TEST(TestExtentsConstructorPreconditions, test_extents_construct_indices) { + auto test_precondition_indices_not_representable = [] { + [[maybe_unused]] auto exts = Kokkos::dextents< std::int8_t, 2 >{ 500, 500 }; + }; + EXPECT_DEATH(test_precondition_indices_not_representable(), ""); + + auto test_precondition_indices_are_negative = [] { + [[maybe_unused]] auto exts = Kokkos::dextents< int, 2 >{ 500, -500 }; + }; + EXPECT_DEATH(test_precondition_indices_are_negative(), ""); +} + +TEST(TestExtentsConstructorPreconditions, test_extents_construct_array) { + auto test_precondition_array_elements_not_representable = [] { + [[maybe_unused]] auto exts = Kokkos::dextents< std::int8_t, 2 >{ std::array{ 500, 500 } }; + }; + EXPECT_DEATH(test_precondition_array_elements_not_representable(), ""); + + auto test_precondition_array_elements_are_negative = [] { + [[maybe_unused]] auto exts = Kokkos::dextents< int, 2 >{ std::array{ 500, -500 } }; + }; + EXPECT_DEATH(test_precondition_array_elements_are_negative(), ""); +} + +#ifdef __cpp_lib_span +TEST(TestExtentsConstructorPreconditions, test_extents_construct_span) { + auto test_precondition_span_elements_not_representable = [] { + auto indices = std::array{ 500, 500 }; + [[maybe_unused]] auto exts = Kokkos::dextents< std::int8_t, 2 >{ std::span{ indices } }; + }; + EXPECT_DEATH(test_precondition_span_elements_not_representable(), ""); + + auto test_precondition_span_elements_are_negative = [] { + auto indices = std::array{ 500, -500 }; + [[maybe_unused]] auto exts = Kokkos::dextents< int, 2 >{ std::span{ indices } }; + }; + EXPECT_DEATH(test_precondition_span_elements_are_negative(), ""); +} +#endif + +TEST(TestExtentsConstructorPreconditions, test_extents_construct_other_extents) { + auto test_precondition_extent_ranks_not_representable = [] { + auto first = Kokkos::dextents< int, 2 >{ 500, 500 }; + [[maybe_unused]] auto exts = Kokkos::dextents< std::int8_t, 2 >{ first }; + }; + EXPECT_DEATH(test_precondition_extent_ranks_not_representable(), ""); +} From ef12e3e720916cc588473a67a0e2e51b4ccd3bd4 Mon Sep 17 00:00:00 2001 From: Nicolas Morales Date: Mon, 16 Jun 2025 10:49:51 +0300 Subject: [PATCH 2/7] #409: only check extents preconditions in C++17 and above mode --- include/experimental/__p0009_bits/extents.hpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/experimental/__p0009_bits/extents.hpp b/include/experimental/__p0009_bits/extents.hpp index c8d8f6b7..9fb3981b 100644 --- a/include/experimental/__p0009_bits/extents.hpp +++ b/include/experimental/__p0009_bits/extents.hpp @@ -440,8 +440,10 @@ template class extents { MDSPAN_INLINE_FUNCTION constexpr explicit extents(OtherIndexTypes... dynvals) noexcept : m_vals(static_cast(dynvals)...) { +#if MDSPAN_HAS_CXX_17 MDSPAN_IMPL_PRECONDITION( detail::all_values_are_nonnegative_and_representable(dynvals...)); +#endif } MDSPAN_TEMPLATE_REQUIRES( @@ -456,9 +458,11 @@ template class extents { MDSPAN_CONDITIONAL_EXPLICIT(N != m_rank_dynamic) constexpr extents(const std::array &exts) noexcept : m_vals(std::move(exts)) { +#if MDSPAN_HAS_CXX_17 MDSPAN_IMPL_PRECONDITION( detail::range_is_nonnegative_and_representable( std::begin(exts), std::end(exts))); +#endif } #ifdef __cpp_lib_span @@ -552,8 +556,10 @@ template class extents { : m_vals(impl_construct_vals_from_extents( std::integral_constant(), std::integral_constant(), other)) { +#if MDSPAN_HAS_CXX_17 MDSPAN_IMPL_PRECONDITION( detail::extent_is_representable(other)); +#endif } // Comparison operator From 11d9a77caeb144d4aee5cfda9fdb95df7f18a47f Mon Sep 17 00:00:00 2001 From: Nicolas Morales Date: Mon, 16 Jun 2025 10:54:00 +0300 Subject: [PATCH 3/7] #409: disable precondition checks for extent in C++ < 17 --- tests/test_extents.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_extents.cpp b/tests/test_extents.cpp index 6a0fe8dc..b265ed5b 100644 --- a/tests/test_extents.cpp +++ b/tests/test_extents.cpp @@ -482,6 +482,7 @@ TEST(TestExtentsCTADStdArray, test_extents_ctad_std_array) { */ #endif +#if MDSPAN_HAS_CXX_17 TEST(TestExtentsConstructorPreconditions, test_extents_construct_indices) { auto test_precondition_indices_not_representable = [] { [[maybe_unused]] auto exts = Kokkos::dextents< std::int8_t, 2 >{ 500, 500 }; @@ -529,3 +530,4 @@ TEST(TestExtentsConstructorPreconditions, test_extents_construct_other_extents) }; EXPECT_DEATH(test_precondition_extent_ranks_not_representable(), ""); } +#endif From 866759379a4ff02a1245cf89af8d6a4ab4f02ccc Mon Sep 17 00:00:00 2001 From: Nicolas Morales Date: Mon, 16 Jun 2025 11:48:43 +0300 Subject: [PATCH 4/7] #409: only check in_range for integral extents arguments; additionally modify in_range implementation to static assert the types are integral --- include/experimental/__p0009_bits/utility.hpp | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/include/experimental/__p0009_bits/utility.hpp b/include/experimental/__p0009_bits/utility.hpp index c33a3a18..d9d808ef 100644 --- a/include/experimental/__p0009_bits/utility.hpp +++ b/include/experimental/__p0009_bits/utility.hpp @@ -198,18 +198,32 @@ MDSPAN_INLINE_FUNCTION constexpr bool cmp_greater_equal(T t, U u) noexcept { template MDSPAN_INLINE_FUNCTION constexpr bool in_range(T t) noexcept { + static_assert(std::is_integral_v && std::is_integral_v); return cmp_greater_equal(t, std::numeric_limits::min()) && cmp_less_equal(t, std::numeric_limits::max()); } template MDSPAN_INLINE_FUNCTION constexpr bool is_nonnegative_and_representable(T t) noexcept { - if constexpr (std::is_signed_v) { - if (t < 0) - return false; - } + // T might not be integral and thus invalid to pass to in_range + // Only check this if we can actually call in_range + if constexpr (std::is_integral_v) + { + if constexpr (std::is_signed_v) { + if (t < 0) + return false; + } - return in_range(t); + return in_range(t); + } else + { + if constexpr (std::is_signed_v) { + if (static_cast(t) < 0) + return false; + } + + return true; + } } template From 8cb9a2c609dab56f4f32bc401a74bada9ac4c124 Mon Sep 17 00:00:00 2001 From: Nicolas Morales Date: Wed, 18 Jun 2025 09:19:55 +0300 Subject: [PATCH 5/7] #409: address minor review comments --- include/experimental/__p0009_bits/utility.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/experimental/__p0009_bits/utility.hpp b/include/experimental/__p0009_bits/utility.hpp index d9d808ef..d3f45c8a 100644 --- a/include/experimental/__p0009_bits/utility.hpp +++ b/include/experimental/__p0009_bits/utility.hpp @@ -217,7 +217,7 @@ MDSPAN_INLINE_FUNCTION constexpr bool is_nonnegative_and_representable(T t) noex return in_range(t); } else { - if constexpr (std::is_signed_v) { + if constexpr (std::is_signed_v) { if (static_cast(t) < 0) return false; } @@ -229,13 +229,13 @@ MDSPAN_INLINE_FUNCTION constexpr bool is_nonnegative_and_representable(T t) noex template MDSPAN_INLINE_FUNCTION constexpr bool all_values_are_representable(Values... values) noexcept { - return ( in_range( values ) && ... && true ); + return ( in_range( values ) && ... ); } template MDSPAN_INLINE_FUNCTION constexpr bool all_values_are_nonnegative_and_representable(Values... values) noexcept { - return ( is_nonnegative_and_representable( values ) && ... && true ); + return ( is_nonnegative_and_representable( values ) && ... ); } template From 24f1bba5ee1a0aa025c53e9a64fa47eddef45f09 Mon Sep 17 00:00:00 2001 From: Nicolas Morales Date: Thu, 19 Jun 2025 10:39:58 +0300 Subject: [PATCH 6/7] #409: adjust condition for checking representability of range --- include/experimental/__p0009_bits/utility.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/experimental/__p0009_bits/utility.hpp b/include/experimental/__p0009_bits/utility.hpp index d3f45c8a..4cc25a73 100644 --- a/include/experimental/__p0009_bits/utility.hpp +++ b/include/experimental/__p0009_bits/utility.hpp @@ -240,8 +240,8 @@ all_values_are_nonnegative_and_representable(Values... values) noexcept { template MDSPAN_INLINE_FUNCTION constexpr bool -range_is_nonnegative_and_representable(ContiguousIterator begin, ContiguousIterator end) noexcept { - for ( auto it = begin; it != end; ++it ) + range_is_nonnegative_and_representable(ContiguousIterator begin, ContiguousIterator end) noexcept { + for ( auto it = begin; it < end; ++it ) { if ( !is_nonnegative_and_representable( *it ) ) return false; From 738f357f9b7639a28617018e9548d6ef1ac894b8 Mon Sep 17 00:00:00 2001 From: Nicolas Morales Date: Wed, 25 Jun 2025 10:40:30 -0400 Subject: [PATCH 7/7] #409: test precondition violation message on host --- tests/test_extents.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test_extents.cpp b/tests/test_extents.cpp index b265ed5b..af9dd333 100644 --- a/tests/test_extents.cpp +++ b/tests/test_extents.cpp @@ -487,24 +487,40 @@ TEST(TestExtentsConstructorPreconditions, test_extents_construct_indices) { auto test_precondition_indices_not_representable = [] { [[maybe_unused]] auto exts = Kokkos::dextents< std::int8_t, 2 >{ 500, 500 }; }; +#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL) EXPECT_DEATH(test_precondition_indices_not_representable(), ""); +#else + EXPECT_DEATH(test_precondition_indices_not_representable(), "all_values_are_nonnegative_and_representable"); +#endif auto test_precondition_indices_are_negative = [] { [[maybe_unused]] auto exts = Kokkos::dextents< int, 2 >{ 500, -500 }; }; +#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL) EXPECT_DEATH(test_precondition_indices_are_negative(), ""); +#else + EXPECT_DEATH(test_precondition_indices_are_negative(), "all_values_are_nonnegative_and_representable"); +#endif } TEST(TestExtentsConstructorPreconditions, test_extents_construct_array) { auto test_precondition_array_elements_not_representable = [] { [[maybe_unused]] auto exts = Kokkos::dextents< std::int8_t, 2 >{ std::array{ 500, 500 } }; }; +#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL) EXPECT_DEATH(test_precondition_array_elements_not_representable(), ""); +#else + EXPECT_DEATH(test_precondition_array_elements_not_representable(), "range_is_nonnegative_and_representable"); +#endif auto test_precondition_array_elements_are_negative = [] { [[maybe_unused]] auto exts = Kokkos::dextents< int, 2 >{ std::array{ 500, -500 } }; }; +#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL) EXPECT_DEATH(test_precondition_array_elements_are_negative(), ""); +#else + EXPECT_DEATH(test_precondition_array_elements_are_negative(), "range_is_nonnegative_and_representable"); +#endif } #ifdef __cpp_lib_span @@ -513,13 +529,21 @@ TEST(TestExtentsConstructorPreconditions, test_extents_construct_span) { auto indices = std::array{ 500, 500 }; [[maybe_unused]] auto exts = Kokkos::dextents< std::int8_t, 2 >{ std::span{ indices } }; }; +#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL) EXPECT_DEATH(test_precondition_span_elements_not_representable(), ""); +#else + EXPECT_DEATH(test_precondition_span_elements_not_representable(), "range_is_nonnegative_and_representable"); +#endif auto test_precondition_span_elements_are_negative = [] { auto indices = std::array{ 500, -500 }; [[maybe_unused]] auto exts = Kokkos::dextents< int, 2 >{ std::span{ indices } }; }; +#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL) EXPECT_DEATH(test_precondition_span_elements_are_negative(), ""); +#else + EXPECT_DEATH(test_precondition_span_elements_are_negative(), "range_is_nonnegative_and_representable"); +#endif } #endif @@ -528,6 +552,10 @@ TEST(TestExtentsConstructorPreconditions, test_extents_construct_other_extents) auto first = Kokkos::dextents< int, 2 >{ 500, 500 }; [[maybe_unused]] auto exts = Kokkos::dextents< std::int8_t, 2 >{ first }; }; +#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL) EXPECT_DEATH(test_precondition_extent_ranks_not_representable(), ""); +#else + EXPECT_DEATH(test_precondition_extent_ranks_not_representable(), "extent_is_representable"); +#endif } #endif