[PATCH v2 1/5] libstdc++: Check prerequisites of layout_*::operator().

[Tomasz Kaminski]

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.grosheintz@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.grosheintz@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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20250701/39f4dfa2/attachment.htm>