On Mon, 26 May 2025 at 15:21, Luc Grosheintz <luc.groshei...@gmail.com> wrote:
>
> [mdspan.layout.left.cons] of N4950 states that this ctor is not
> noexcept. Since, all other ctors of layout_left, layout_right or
> layout_stride are noexcept, the choice was made, based on
> [res.on.exception.handling], to make this ctor noexcept.
>
> Two other major implementations of the STL make the same choice.
"standard library implementations" not "implementations of the STL",
since mdspan is not part of the "STL" designed by Stepanov.
>
> libstdc++-v3/ChangeLog:
>
> * include/std/mdspan: Strengthen the exception
> guarantees of layout_left::mapping(layout_stride::mapping).
> * testsuite/23_containers/mdspan/layouts/ctors.cc:
> Simplify tests to reflect the change.
>
> Signed-off-by: Luc Grosheintz <luc.groshei...@gmail.com>
> ---
> libstdc++-v3/include/std/mdspan | 6 +++-
> .../23_containers/mdspan/layouts/ctors.cc | 33 ++++++++-----------
> 2 files changed, 19 insertions(+), 20 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/mdspan b/libstdc++-v3/include/std/mdspan
> index 33ad5070a37..2a7f0452cd7 100644
> --- a/libstdc++-v3/include/std/mdspan
> +++ b/libstdc++-v3/include/std/mdspan
> @@ -561,10 +561,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> : mapping(__other.extents(), __mdspan::__internal_ctor{})
> { }
>
> + // [mdspan.layout.left.cons] of N4950 states that this ctor is not
> + // noexcept. Since, all other ctors of layout_left, layout_right or
> + // layout_stride are noexcept, the choice was made, based on
> + // [res.on.exception.handling], to make this ctor noexcept.
The comment seems a bit verbose, if we wrote four lines every time we
strengthen a noexcept-specifier based on [res.on.exception.handling]
we'd have no room left for code ;-)
I think the git commit message already contains this information, so
if anybody is curious why we made it noexcept they can 'git blame' and
find the rationale there. Or to be explicit, a single line saying "//
noexcept for consistency with other layouts" seems good enough.
I like how MSVC STL does this, which is to mark EVERY strengthened
noexcept with:
/* strengthened */
I can make these changes locally and push the patch, thanks.
> template<typename _OExtents>
> requires (is_constructible_v<extents_type, _OExtents>)
> constexpr explicit(extents_type::rank() > 0)
> - mapping(const layout_stride::mapping<_OExtents>& __other)
> + mapping(const layout_stride::mapping<_OExtents>& __other) noexcept
> : mapping(__other.extents(), __mdspan::__internal_ctor{})
> { __glibcxx_assert(*this == __other); }
>
> diff --git a/libstdc++-v3/testsuite/23_containers/mdspan/layouts/ctors.cc
> b/libstdc++-v3/testsuite/23_containers/mdspan/layouts/ctors.cc
> index 2507eeaf7a1..6f813e9877b 100644
> --- a/libstdc++-v3/testsuite/23_containers/mdspan/layouts/ctors.cc
> +++ b/libstdc++-v3/testsuite/23_containers/mdspan/layouts/ctors.cc
> @@ -362,30 +362,24 @@ namespace from_stride
>
> template<typename Layout, typename Extents, typename OExtents>
> constexpr void
> - verify_convertible(OExtents oexts)
> + verify_nothrow_convertible(OExtents oexts)
> {
> using Mapping = typename Layout::mapping<Extents>;
> using OMapping = std::layout_stride::mapping<OExtents>;
>
> constexpr auto other = OMapping(oexts,
> strides(Mapping(Extents(oexts))));
> - if constexpr (std::is_same_v<Layout, std::layout_right>)
> - ::verify_nothrow_convertible<Mapping>(other);
> - else
> - ::verify_convertible<Mapping>(other);
> + ::verify_nothrow_convertible<Mapping>(other);
> }
>
> template<typename Layout, typename Extents, typename OExtents>
> constexpr void
> - verify_constructible(OExtents oexts)
> + verify_nothrow_constructible(OExtents oexts)
> {
> using Mapping = typename Layout::mapping<Extents>;
> using OMapping = std::layout_stride::mapping<OExtents>;
>
> constexpr auto other = OMapping(oexts,
> strides(Mapping(Extents(oexts))));
> - if constexpr (std::is_same_v<Layout, std::layout_right>)
> - ::verify_nothrow_constructible<Mapping>(other);
> - else
> - ::verify_constructible<Mapping>(other);
> + ::verify_nothrow_constructible<Mapping>(other);
> }
>
> template<typename Layout>
> @@ -404,31 +398,32 @@ namespace from_stride
> typename Layout::mapping<std::extents<int, 2>>,
> std::layout_stride::mapping<std::extents<int, 1>>>();
>
> - verify_convertible<Layout, std::extents<int>>(std::extents<int>{});
> + verify_nothrow_convertible<Layout, std::extents<int>>(
> + std::extents<int>{});
>
> - verify_convertible<Layout, std::extents<unsigned int>>(
> + verify_nothrow_convertible<Layout, std::extents<unsigned int>>(
> std::extents<int>{});
>
> // Rank == 0 doesn't check IndexType for convertibility.
> - verify_convertible<Layout, std::extents<int>>(
> + verify_nothrow_convertible<Layout, std::extents<int>>(
> std::extents<unsigned int>{});
>
> - verify_constructible<Layout, std::extents<int, 3>>(
> + verify_nothrow_constructible<Layout, std::extents<int, 3>>(
> std::extents<int, 3>{});
>
> - verify_constructible<Layout, std::extents<unsigned int, 3>>(
> + verify_nothrow_constructible<Layout, std::extents<unsigned int, 3>>(
> std::extents<int, 3>{});
>
> - verify_constructible<Layout, std::extents<int, 3>>(
> + verify_nothrow_constructible<Layout, std::extents<int, 3>>(
> std::extents<unsigned int, 3>{});
>
> - verify_constructible<Layout, std::extents<int, 3, 5>>(
> + verify_nothrow_constructible<Layout, std::extents<int, 3, 5>>(
> std::extents<int, 3, 5>{});
>
> - verify_constructible<Layout, std::extents<unsigned int, 3, 5>>(
> + verify_nothrow_constructible<Layout, std::extents<unsigned int, 3, 5>>(
> std::extents<int, 3, 5>{});
>
> - verify_constructible<Layout, std::extents<int, 3, 5>>(
> + verify_nothrow_constructible<Layout, std::extents<int, 3, 5>>(
> std::extents<unsigned int, 3, 5>{});
> return true;
> }
> --
> 2.49.0
>
