On Fri, Oct 10, 2025 at 11:53 PM Tomasz Kaminski <[email protected]> wrote: > On Fri, Oct 10, 2025 at 5:21 PM Yuao Ma <[email protected]> wrote: >> >> Hi Tomasz, >> >> On Fri, Oct 10, 2025 at 10:52 PM Tomasz Kaminski <[email protected]> wrote: >> > >> > Hi, >> > >> > Firstly, just to confirm, do you have a copyright assignment for >> > GCC in place (or are covered by a corporate assignment)? >> > >> > If not, please complete that process, or contribute under the DCO >> > terms, see https://gcc.gnu.org/contribute.html#legal >> > >> > If the patch is being contributed under the DCO, please resubmit it >> > with a Signed-off-by tag as per the first link :) >> > >> >> Actually, I'm already a committer - you can find my DCO in the >> MAINTAINERS file : ) >> I mostly contributed to gfortran as part of my GSOC project before >> this, this is my first time working on libstdc++. > > Thanks, no need for the Signed-off-by tag. >> >> >> > Secondly: >> > We will need to have two overload of address function, to preserve >> > the qualification: >> > In atomic_ref_bse<const _TP>, the existing one should return const _Tp* >> > #if __glibcxx_atomic_ref >= 202411L >> > _GLIBCXX_ALWAYS_INLINE constexpr const _Tp* >> > address() const noexcept >> > { return _M_ptr; } >> > #endif // __glibcxx_atomic_ref >= 202411L >> > >> > And we need to also define in in atomic_ref_base<_Tp>, this will hide one >> > from ato >> > #if __glibcxx_atomic_ref >= 202411L >> > _GLIBCXX_ALWAYS_INLINE constexpr _Tp* >> > address() const noexcept >> > { return _M_ptr; } >> > #endif // __glibcxx_atomic_ref >= 202411L >> > >> >> That was indeed my oversight. It's fixed in the new patch. >> >> > For the test I would split them into two levels: >> > template <typename T> >> > void test() >> > { >> > T x(T(42)); >> > const std::atomic_ref<T> a(x); >> > >> > static_assert(noexcept(a.address())); >> > VERIFY( std::addressof(x) == a.address() ); >> > // I would also add a test that decltype(a.address()) is T*: >> > // that will detect above issue. >> > static_assert( std::is_same_v<decltype(a.address()), T*> ); >> > } >> > >> > >> > template<typename T> >> > void test_cv() >> > { >> > test<T>(); >> > test<const T>(); >> > test<volatile T>(); >> > test<const volatile T>(); >> > } >> > >> > And then in main, you could just use: >> > test<x>; >> > >> >> This really cleans up the test case! Done. > > LGTM. This still needs to be reviewed and approved by Jonathan, before it can > be merged.
Thank you for the prompt review! Hi Jonathan, can you please take a look at this patch as well? Thanks! Yuao
