On 7/1/25 12:30, Tomasz Kaminski wrote:
Hi,
More of the review will be later, but I have noticed that you have added
preconditions checks
to the layouts, and then avoid checking them inside the operator[] of the
mdspan. This is general
sounds good.
However, the precondition on mdspan::operator[] is now hardened:
https://eel.is/c++draft/views.multidim#mdspan.mdspan.members-3.
This implies that breaking this precondition is a contract violation, and
thus the user may expect a contract violation handler to be invoked
for it. Amongst the information provided to the handler via
contract_violation object (
https://eel.is/c++draft/support.contract.violation)
is source_location, that includes the name. Given that I think we want to
always check the extents in operator[] of mdspan, and thus remove
the checks from layout (to avoid duplication).
Hope, the above makes sense.
Thank you, makes sense. I've always been reading N4950 when implementing
the C++23 parts of mdspan. However, it makes sense to already prepare for
the hardening coming in C++26.
If I understand correctly, you'd like to do a second round of reviewing.
I'll wait with v3 until you clearly tell me to submit the next iteration.
Regards,
Tomasz
On Fri, Jun 27, 2025 at 11:12 AM Luc Grosheintz <luc.groshei...@gmail.com>
wrote:
Previously the prerequisite that the arguments passed to operator() are
a multi-dimensional index (of extents()) was not checked.
This commit adds the __glibcxx_asserts and the required tests.
libstdc++-v3/ChangeLog:
* include/std/mdspan: Check prerequisites of
layout_*::operator().
* testsuite/23_containers/mdspan/layouts/class_mandate_neg.cc:
Add tests for prerequisites.
Signed-off-by: Luc Grosheintz <luc.groshei...@gmail.com>
---
libstdc++-v3/include/std/mdspan | 4 +++
.../mdspan/layouts/class_mandate_neg.cc | 26 +++++++++++++++++++
2 files changed, 30 insertions(+)
diff --git a/libstdc++-v3/include/std/mdspan
b/libstdc++-v3/include/std/mdspan
index c72a64094b7..39d02ac08df 100644
--- a/libstdc++-v3/include/std/mdspan
+++ b/libstdc++-v3/include/std/mdspan
@@ -441,6 +441,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_IndexType __mult = 1;
auto __update = [&, __pos = 0u](_IndexType __idx) mutable
{
+ __glibcxx_assert(cmp_less(__idx, __exts.extent(__pos)));
__res += __idx * __mult;
__mult *= __exts.extent(__pos);
++__pos;
@@ -651,6 +652,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
auto __update = [&, __pos = __exts.rank()](_IndexType) mutable
{
--__pos;
+ __glibcxx_assert(cmp_less(__ind_arr[__pos],
+ __exts.extent(__pos)));
__res += __ind_arr[__pos] * __mult;
__mult *= __exts.extent(__pos);
};
@@ -822,6 +825,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
auto __update = [&, __pos = 0u](_IndexType __idx) mutable
{
+ __glibcxx_assert(cmp_less(__idx,
__m.extents().extent(__pos)));
__res += __idx * __m.stride(__pos++);
};
(__update(__indices), ...);
diff --git
a/libstdc++-v3/testsuite/23_containers/mdspan/layouts/class_mandate_neg.cc
b/libstdc++-v3/testsuite/23_containers/mdspan/layouts/class_mandate_neg.cc
index 7091153daba..f5aab22aff0 100644
---
a/libstdc++-v3/testsuite/23_containers/mdspan/layouts/class_mandate_neg.cc
+++
b/libstdc++-v3/testsuite/23_containers/mdspan/layouts/class_mandate_neg.cc
@@ -45,4 +45,30 @@ auto b5 = B<5, std::layout_stride,
std::layout_right>(); // { dg-error "require
auto b6 = B<6, std::layout_stride, std::layout_left>(); // { dg-error
"required from" }
auto b7 = B<7, std::layout_stride, std::layout_stride>(); // { dg-error
"required from" }
+template<typename Layout>
+ constexpr bool
+ test_linear_index_0d()
+ {
+ auto m = typename Layout::mapping<std::extents<int, 0>>{};
+ (void) m(0); // { dg-error "expansion of" }
+ return true;
+ }
+static_assert(test_linear_index_0d<std::layout_left>()); // { dg-error
"expansion of" }
+static_assert(test_linear_index_0d<std::layout_right>()); // { dg-error
"expansion of" }
+static_assert(test_linear_index_0d<std::layout_stride>()); // { dg-error
"expansion of" }
+
+template<typename Layout>
+ constexpr bool
+ test_linear_index_3d()
+ {
+ auto m = typename Layout::mapping<std::extents<int, 3, 5, 7>>{};
+ (void) m(2, 5, 5); // { dg-error "expansion of" }
+ return true;
+ }
+static_assert(test_linear_index_3d<std::layout_left>()); // { dg-error
"expansion of" }
+static_assert(test_linear_index_3d<std::layout_right>()); // { dg-error
"expansion of" }
+static_assert(test_linear_index_3d<std::layout_stride>()); // { dg-error
"expansion of" }
+
// { dg-prune-output "must be representable as index_type" }
+// { dg-prune-output "non-constant condition for static assertion" }
+// { dg-prune-output "__glibcxx_assert" }
--
2.49.0