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. > > Yuao >
