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. 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 > >
