On Tue, Sep 30, 2025 at 10:45 AM Luc Grosheintz <[email protected]>
wrote:
>
>
> On 9/30/25 10:29 AM, Tomasz Kaminski wrote:
> > I have reviewed both patches, and posted v5 for layout_left_padded. I am
> > running a full test suite on them
> > will post them when they are finished.
> >
> > One suggestion, I would consider in future. Instead of befriending
> > __dynamic_extents, __get_first_stride, __static_extents,
> > I think we could have a internal __storage function:
> > template<typename _ExtentsOrMapping>
> > const _ExtentsStorage& __storage(const _ExtentsOrMapping& __eom)
> > {
> > if constexpr (requires { __eom._M_extents})
> > return __mdspan::__storage(__eom._M_extents);
> > else
> > return __eom._M_storage;
> > }
> >
>
> I like the idea of removing the numerous friends. I'm not
> fully sold on the idea that sometimes it calls the storage
> of the object (i.e. for padded layouts and extents); and
> sometimes is returns the storage of _M_extents. It's likely
> easily fixed by giving everything an _M_storage (as you
> propose further down. I feels like it's made to make other
> code convenient, but right now (and that often changes with
> time) I don't have a good way of thinking of __storage without
> remembering if-conditions.
>
For the moment we need that for extents and padded_layouts,
and they both have _M_storage members, so we do not really need
the if constexpr.
>
> Making layout_left and layout_right DRYer by using a similar
> storage object as the padded layouts is also appealing.
>
>
> > This way __static_extents can just call __strorage(x)._M_static_extents,
> > and so on.
> > Similarly, when calling _M_equal in padded layouts operator== we could
> just
> > call
> > _M_equal(__mdspan::__storage(__other)).
> >
> > And __get_static_stride will become
> __mdspan::__storage(x)._M_padstride();
> >
> > We may consider having for layout_stride having an internal private
> class:
> > struct _Storage { _M_extents, __strides };
> > _Storage _M_storage
> >
> > WDYT?
> >
> >
> >
> >
> > On Mon, Sep 29, 2025 at 8:03 AM Luc Grosheintz <[email protected]
> >
> > wrote:
> >
> >> Changes since v3:
> >> - Improve comments in version.def
> >> - Avoid check statically asserted preconditions:
> >> + layout_left::mapping(LeftPaddedMapping)
> >> + layout_right::mapping(RightPaddedMapping)
> >> + _PaddedStorage(const _Extents&)
> >> + _PaddedStorage(const _LayoutSame&)
> >> + _PaddedStorage(const layout_stride&)
> >> + _PaddedStorage(const SamePaddedMapping&)
> >>
> >> - Use `constexpr static const size_t` in traits.
> >> - Reduce constraints in _PaddedStorage.
> >> - Use tag-types to completely eliminate constraints in
> _PaddedStorage;
> >> and
> >> simplify the traits.
> >> - Rename and inline:
> >> + _S_static_padextent
> >> + _M_padextent
> >> + _M_padstride
> >>
> >> - Introduce __index_type_cast that checks the representability
> >> and non-negativity conditions, and casts to index_type.
> >> - Simplify reverse.
> >> - Lift operator== to _PaddedStorage (as _M_equal).
> >> - Restructure __valid_padded_size to use _S_static_padextent.
> >> - Move is_padded_layout from ctors.cc to padded_traits.h
> >> - Eliminate _S_static_stride in the userfacing mappings.
> >> - Also make the multi arg ctor explicit.
> >> - Flatten else { if ... }.
> >> - Use __cplusplus > 202302L in tests.
> >> - Restructure _M_required_span_size
> >> - Add error message to a static_assert.
> >> - Move checking if static padding values are representable up.
> >> - Remove unneeded member initilizer in _PaddedStorage.
> >> - Fix formatting and whitespace.
> >>
> >> Not strictly requested changes:
> >>
> >> - When checking __valid_padded_size, use the fact that we
> >> can't handle index_type >= size_t. Therefore, only check if the
> padded
> >> size fits into index_type.
> >>
> >> - Include the bugzilla issue number in the commit messages.
> >> - Remove stray #include <iostream>
> >> - Rename make_expected to make_array.
> >>
> >> Luc Grosheintz (2):
> >> libstdc++: Implement std::layout_left_padded [PR110352].
> >> libstdc++: Implement layout_right_padded [PR110352].
> >>
> >> libstdc++-v3/include/bits/version.def | 10 +
> >> libstdc++-v3/include/bits/version.h | 9 +
> >> libstdc++-v3/include/std/mdspan | 903 +++++++++++++++++-
> >> libstdc++-v3/src/c++23/std.cc.in | 9 +-
> >> .../mdspan/layouts/class_mandate_neg.cc | 1 +
> >> .../23_containers/mdspan/layouts/ctors.cc | 60 +-
> >> .../23_containers/mdspan/layouts/empty.cc | 21 +-
> >> .../23_containers/mdspan/layouts/mapping.cc | 109 ++-
> >> .../23_containers/mdspan/layouts/padded.cc | 677 +++++++++++++
> >> .../mdspan/layouts/padded_neg.cc | 352 +++++++
> >> .../mdspan/layouts/padded_traits.h | 149 +++
> >> 11 files changed, 2283 insertions(+), 17 deletions(-)
> >> create mode 100644
> >> libstdc++-v3/testsuite/23_containers/mdspan/layouts/padded.cc
> >> create mode 100644
> >> libstdc++-v3/testsuite/23_containers/mdspan/layouts/padded_neg.cc
> >> create mode 100644
> >> libstdc++-v3/testsuite/23_containers/mdspan/layouts/padded_traits.h
> >>
> >> --
> >> 2.50.1
> >>
> >>
> >
>
>
