Thanks for working on the patches, they look solid, comments.
Could you prepare a separate patch to fix initialization
default-initialization of extents,
that you have noticed, standard requires them to be value-initialized, and
add corresponding test?
Similarly, we have test for default constructor of stride_mapping, I would
add them for other layouts,
and check:
* all extents being static -> depends on mapping
* first extent being dynamic, and rest static -> all strides are zero
* middle extent being dynamic
For the stride and product computation, we should perform them in
Extent::size_type, not index_type.
The latter may be signed, and we may hit UB in multiplying non-zero
extents, before reaching the zero.
For is_exhaustive question, I will write to LWG reflector to ask authors,
and see what their opinion is.
Will keep you posted.
In a lot of tests we are doing, where I believe we could skip template
parameters, and deduce it for argument.
verify_nothrow_convertible<Layout, std::extents<int>>(
+ std::extents<int>{});
Could you look into doing it?
Regards,
Tomasz
On Thu, May 22, 2025 at 2:21 PM Tomasz Kaminski <tkami...@redhat.com> wrote:
>
>
> On Wed, May 21, 2025 at 4:21 PM Luc Grosheintz <luc.groshei...@gmail.com>
> wrote:
>
>> It's missing the "registration" of the three new classes in
>> std.cc.in.
>>
> Please remember to add it in next revisions.
>
>>
>> On 5/21/25 11:40, Luc Grosheintz wrote:
>> > Follows up on:
>> > https://gcc.gnu.org/pipermail/libstdc++/2025-May/061535.html
>> >
>> > To improve naming conventions, this series includes three new commits:
>> > * Two commits to rename _ExtentsStorage::_M_dynamic_extents, and
>> > extents::_M_dynamic_extents.
>> > * One commit to cleanup whitespace errors in extents.
>> >
>> > The changes to the existing commits are:
>> > * Fix division by zero bug.
>> > * Rename subextents -> extents.
>> > * Default arguments for __{static,dynamic}_extents.
>> > * Default argument for __static_quotient.
>> > * Four times: use range-based for.
>> > * Eliminate __has_static_zero
>> > * Short-circuit in __static_quotient.
>> > * Optimize __exts_prod for rank == rank_dynamic.
>> >
>> > This review suggestion was intentionally skipped:
>> > * Inline helper of __exts_prod, because with the additional
>> > optimization for rank == rank_dynamic, having two separate
>> > functions makes the highlevel structure a little bit more
>> > obvious. Additionally, there's numerous changes planned that
>> > might make one of the two functions much more verbose.
>> >
>> > Luc Grosheintz (9):
>> > libstdc++: Rename _ExtentsStorage::_M_dynamic_extents.
>> > libstdc++: Rename extents::_M_dynamic_extents.
>> > libstdc++: Cleanup formatting in mdspan.
>> > libstdc++: Implement layout_left from mdspan.
>> > libstdc++: Add tests for layout_left.
>> > libstdc++: Implement layout_right from mdspan.
>> > libstdc++: Add tests for layout_right.
>> > libstdc++: Implement layout_stride from mdspan.
>> > libstdc++: Add tests for layout_stride.
>> >
>> > libstdc++-v3/include/std/mdspan | 692 +++++++++++++++++-
>> > .../mdspan/layouts/class_mandate_neg.cc | 42 ++
>> > .../23_containers/mdspan/layouts/ctors.cc | 401 ++++++++++
>> > .../23_containers/mdspan/layouts/mapping.cc | 569 ++++++++++++++
>> > .../23_containers/mdspan/layouts/stride.cc | 494 +++++++++++++
>> > 5 files changed, 2185 insertions(+), 13 deletions(-)
>> > create mode 100644
>> libstdc++-v3/testsuite/23_containers/mdspan/layouts/class_mandate_neg.cc
>> > create mode 100644
>> libstdc++-v3/testsuite/23_containers/mdspan/layouts/ctors.cc
>> > create mode 100644
>> libstdc++-v3/testsuite/23_containers/mdspan/layouts/mapping.cc
>> > create mode 100644
>> libstdc++-v3/testsuite/23_containers/mdspan/layouts/stride.cc
>> >
>>
>>
