[PATCH v4 1/1] libstdc++: Implement default_accessor from mdspan.
Luc Grosheintz
luc.grosheintz@gmail.com
Fri Jun 27 11:17:46 GMT 2025
On 6/27/25 11:44, Jonathan Wakely wrote:
> On Fri, 27 Jun 2025 at 10:39, Tomasz Kaminski <tkaminsk@redhat.com> wrote:
>>
>> Also, for single patch (not-patch series), you do not need to have [PATCH 0/N], simple [PATCH] and then [PATCH v2] also works.
>
> Yeah, sending a 0/N cover letter is only useful to describe what a
> multi-part patch series does. For a single patch, you should be
> describing what it does in that patch itself, and a cover letter just
> adds noise.
>
Makes sense & I've found the place I can add comments, so there's
no need for the cover-letter anymore.
>
>>
>> On Fri, Jun 27, 2025 at 11:11 AM Tomasz Kaminski <tkaminsk@redhat.com> wrote:
>>>
>>>
>>>
>>> On Fri, Jun 27, 2025 at 11:06 AM Luc Grosheintz <luc.grosheintz@gmail.com> wrote:
>>>>
>>>> libstdc++-v3/ChangeLog:
>>>>
>>>> * include/std/mdspan (default_accessor): New class.
>>>> * src/c++23/std.cc.in: Register default_accessor.
>>>> * testsuite/23_containers/mdspan/accessors/default.cc: New test.
>>>> * testsuite/23_containers/mdspan/accessors/default_neg.cc: New test.
>>>>
>>>> Signed-off-by: Luc Grosheintz <luc.grosheintz@gmail.com>
>>>> ---
>>>> libstdc++-v3/include/std/mdspan | 31 ++++++++
>>>> libstdc++-v3/src/c++23/std.cc.in | 3 +-
>>>> .../23_containers/mdspan/accessors/default.cc | 72 +++++++++++++++++++
>>>> .../mdspan/accessors/default_neg.cc | 23 ++++++
>>>> 4 files changed, 128 insertions(+), 1 deletion(-)
>>>> create mode 100644 libstdc++-v3/testsuite/23_containers/mdspan/accessors/default.cc
>>>> create mode 100644 libstdc++-v3/testsuite/23_containers/mdspan/accessors/default_neg.cc
>>>>
>>>> diff --git a/libstdc++-v3/include/std/mdspan b/libstdc++-v3/include/std/mdspan
>>>> index 6dc2441f80b..c72a64094b7 100644
>>>> --- a/libstdc++-v3/include/std/mdspan
>>>> +++ b/libstdc++-v3/include/std/mdspan
>>>> @@ -1004,6 +1004,37 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>> [[no_unique_address]] _S_strides_t _M_strides;
>>>> };
>>>>
>>>> + template<typename _ElementType>
>>>> + struct default_accessor
>>>> + {
>>>> + static_assert(!is_array_v<_ElementType>,
>>>> + "ElementType must not be an array type");
>>>> + static_assert(!is_abstract_v<_ElementType>,
>>>> + "ElementType must not be an abstract class type");
>>>> +
>>>> + using offset_policy = default_accessor;
>>>> + using element_type = _ElementType;
>>>> + using reference = element_type&;
>>>> + using data_handle_type = element_type*;
>>>> +
>>>> + constexpr
>>>> + default_accessor() noexcept = default;
>>>> +
>>>> + template<typename _OElementType>
>>>> + requires is_convertible_v<_OElementType(*)[], element_type(*)[]>
>>>> + constexpr
>>>> + default_accessor(default_accessor<_OElementType>) noexcept
>>>> + { }
>>>> +
>>>> + constexpr reference
>>>> + access(data_handle_type __p, size_t __i) const noexcept
>>>> + { return __p[__i]; }
>>>> +
>>>> + constexpr data_handle_type
>>>> + offset(data_handle_type __p, size_t __i) const noexcept
>>>> + { return __p + __i; }
>>>> + };
>>>> +
>>>> _GLIBCXX_END_NAMESPACE_VERSION
>>>> }
>>>> #endif
>>>> diff --git a/libstdc++-v3/src/c++23/std.cc.in b/libstdc++-v3/src/c++23/std.cc.in
>>>> index 9336118f5d9..e692caaa5f9 100644
>>>> --- a/libstdc++-v3/src/c++23/std.cc.in
>>>> +++ b/libstdc++-v3/src/c++23/std.cc.in
>>>> @@ -1850,7 +1850,8 @@ export namespace std
>>>> using std::layout_left;
>>>> using std::layout_right;
>>>> using std::layout_stride;
>>>> - // FIXME layout_left_padded, layout_right_padded, default_accessor and mdspan
>>>> + using std::default_accessor;
>>>> + // FIXME layout_left_padded, layout_right_padded, aligned_accessor and mdspan
>>>> }
>>>> #endif
>>>>
>>>> diff --git a/libstdc++-v3/testsuite/23_containers/mdspan/accessors/default.cc b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/default.cc
>>>> new file mode 100644
>>>> index 00000000000..ecccda2b68e
>>>> --- /dev/null
>>>> +++ b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/default.cc
>>>> @@ -0,0 +1,72 @@
>>>> +// { dg-do run { target c++23 } }
>>>> +#include <mdspan>
>>>> +
>>>> +#include <testsuite_hooks.h>
>>>> +
>>>> +constexpr size_t dyn = std::dynamic_extent;
>>>> +
>>>> +template<typename Accessor>
>>>> + constexpr void
>>>> + test_accessor_policy()
>>>> + {
>>>> + static_assert(std::copyable<Accessor>);
>>>> + static_assert(std::is_nothrow_move_constructible_v<Accessor>);
>>>> + static_assert(std::is_nothrow_move_assignable_v<Accessor>);
>>>> + static_assert(std::is_nothrow_swappable_v<Accessor>);
>>>> + }
>>>> +
>>>> +constexpr bool
>>>> +test_access()
>>>> +{
>>>> + std::default_accessor<double> accessor;
>>>> + std::array<double, 5> a{10, 11, 12, 13, 14};
>>>> + VERIFY(accessor.access(a.data(), 0) == 10);
>>>> + VERIFY(accessor.access(a.data(), 4) == 14);
>>>> + return true;
>>>> +}
>>>> +
>>>> +constexpr bool
>>>> +test_offset()
>>>> +{
>>>> + std::default_accessor<double> accessor;
>>>> + std::array<double, 5> a{10, 11, 12, 13, 14};
>>>> + VERIFY(accessor.offset(a.data(), 0) == a.data());
>>>> + VERIFY(accessor.offset(a.data(), 4) == a.data() + 4);
>>>> + return true;
>>>> +}
>>>> +
>>>> +class Base
>>>> +{ };
>>>> +
>>>> +class Derived : public Base
>>>> +{ };
>>>> +
>>>> +constexpr void
>>>> +test_ctor()
>>>> +{
>>>> + static_assert(std::is_nothrow_constructible_v<std::default_accessor<double>,
>>>> + std::default_accessor<double>>);
>>>
>>> Hi, sorry for being unclear before, and resulting in another patch.
>>> I would like to see a positive test case that cost-adjustment are allowed, i.e.:
>>> + static_assert(std::is_convertible_v<std::default_accessor<double>,
>>> + std::default_accessor<const double>>);
>>> And similar for Derived. This is important, as it allows passing mdspan<T>
>>> to function accepting mdspan<const T>.
I'm sorry, I was hasty reading your first review.
>>>>
>>>> + static_assert(std::is_convertible_v<std::default_accessor<double>,
>>>> + std::default_accessor<double>>);
>>>> + static_assert(!std::is_constructible_v<std::default_accessor<char>,
>>>> + std::default_accessor<int>>);
>>>> + static_assert(!std::is_constructible_v<std::default_accessor<int>,
>>>> + std::default_accessor<unsigned int>>);
>>>> + static_assert(!std::is_constructible_v<std::default_accessor<Base>,
>>>> + std::default_accessor<Derived>>);
>>>> + static_assert(!std::is_constructible_v<std::default_accessor<Derived>,
>>>> + std::default_accessor<Base>>);
>>>> +
>>>> +}
>>>> +
>>>> +int
>>>> +main()
>>>> +{
>>>> + test_accessor_policy<std::default_accessor<double>>();
>>>> + test_access();
>>>> + static_assert(test_access());
>>>> + test_offset();
>>>> + static_assert(test_offset());
>>>> + test_ctor();
>>>> + return 0;
>>>> +}
>>>> diff --git a/libstdc++-v3/testsuite/23_containers/mdspan/accessors/default_neg.cc b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/default_neg.cc
>>>> new file mode 100644
>>>> index 00000000000..f8da2b569ca
>>>> --- /dev/null
>>>> +++ b/libstdc++-v3/testsuite/23_containers/mdspan/accessors/default_neg.cc
>>>> @@ -0,0 +1,23 @@
>>>> +// { dg-do compile { target c++23 } }
>>>> +#include<mdspan>
>>>> +
>>>> +std::default_accessor<int[3]> a; // { dg-error "required from here" }
>>>> +
>>>> +class AbstractBase
>>>> +{
>>>> + virtual void
>>>> + foo() const = 0;
>>>> +};
>>>> +
>>>> +class Derived : public AbstractBase
>>>> +{
>>>> + void
>>>> + foo() const override
>>>> + { }
>>>> +};
>>>> +
>>>> +std::default_accessor<Derived> b_ok;
>>>> +std::default_accessor<AbstractBase> b_err; // { dg-error "required from here"}
>>>> +
>>>> +// { dg-prune-output "ElementType must not be an array type" }
>>>> +// { dg-prune-output "ElementType must not be an abstract" }
>>>> --
>>>> 2.49.0
>>>>
>
More information about the Libstdc++
mailing list