libstdc++: Make atomic notify_one and notify_all non-const
Commit Message
<recording this here for future reference>
PR102994 "atomics: std::atomic<ptr>::wait is not marked const" raises the
issue that the current libstdc++ implementation marks the notify members
const, the implementation strategy used by libstdc++, as well as libc++
and the Microsoft STL, do not require the atomic to be mutable (it is hard
to conceive of a desirable implementation approach that would require it).
The original paper proposing the wait/notify functionality for atomics
(p1185) also had these members marked const for the first three revisions,
but that was changed without explanation in r3 and subsequent revisions of
the paper.
After raising the issue to the authors of p1185 and the author of the
libc++ implementation, the consensus seems to be "meh, it's harmless" so
there seems little appetite for an LWG issue to revisit the subject.
This patch changes the libstdc++ implementation to be in agreement with
the standard by removing const from those notify_one/notify_all members.
libstdc++-v3/ChangeLog:
* include/bits/atomic_base.h (atomic_flag::notify_one,
notify_all): Remove const qualification.
(__atomic_base::notify_one, notify_all): Likewise.
* include/std/atomic (atomic<bool>::notify_one, notify_all):
Likewise.
(atomic::notify_one, notify_all): Likewise.
(atomic<T*>::notify_one, notify_all): Likewise.
(atomic_notify_one, atomic_notify_all): Likewise.
* testsuite/29_atomics/atomic/wait_notify/102994.cc: Adjust test
to account for change in notify_one/notify_all signature.
Tested x86_64-pc-linux-gnu.
Comments
On Fri, 11 Feb 2022 at 17:40, Thomas Rodgers via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> <recording this here for future reference>
> PR102994 "atomics: std::atomic<ptr>::wait is not marked const" raises the
> issue that the current libstdc++ implementation marks the notify members
> const, the implementation strategy used by libstdc++, as well as libc++
> and the Microsoft STL, do not require the atomic to be mutable (it is hard
> to conceive of a desirable implementation approach that would require it).
> The original paper proposing the wait/notify functionality for atomics
> (p1185) also had these members marked const for the first three revisions,
> but that was changed without explanation in r3 and subsequent revisions of
> the paper.
>
> After raising the issue to the authors of p1185 and the author of the
> libc++ implementation, the consensus seems to be "meh, it's harmless" so
> there seems little appetite for an LWG issue to revisit the subject.
>
> This patch changes the libstdc++ implementation to be in agreement with
> the standard by removing const from those notify_one/notify_all members.
>
> libstdc++-v3/ChangeLog:
Might as well add a "PR libstdc++/102994" here to the bug gets updated
automatically.
OK for trunk with that change.
> * include/bits/atomic_base.h (atomic_flag::notify_one,
> notify_all): Remove const qualification.
> (__atomic_base::notify_one, notify_all): Likewise.
> * include/std/atomic (atomic<bool>::notify_one, notify_all):
> Likewise.
> (atomic::notify_one, notify_all): Likewise.
> (atomic<T*>::notify_one, notify_all): Likewise.
> (atomic_notify_one, atomic_notify_all): Likewise.
> * testsuite/29_atomics/atomic/wait_notify/102994.cc: Adjust test
> to account for change in notify_one/notify_all signature.
>
> Tested x86_64-pc-linux-gnu.
Committed to trunk, backported to releases/gcc-11.
On Fri, Feb 11, 2022 at 12:22 PM Jonathan Wakely <jwakely@redhat.com> wrote:
> On Fri, 11 Feb 2022 at 17:40, Thomas Rodgers via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
> >
> > <recording this here for future reference>
> > PR102994 "atomics: std::atomic<ptr>::wait is not marked const" raises the
> > issue that the current libstdc++ implementation marks the notify members
> > const, the implementation strategy used by libstdc++, as well as libc++
> > and the Microsoft STL, do not require the atomic to be mutable (it is
> hard
> > to conceive of a desirable implementation approach that would require
> it).
> > The original paper proposing the wait/notify functionality for atomics
> > (p1185) also had these members marked const for the first three
> revisions,
> > but that was changed without explanation in r3 and subsequent revisions
> of
> > the paper.
> >
> > After raising the issue to the authors of p1185 and the author of the
> > libc++ implementation, the consensus seems to be "meh, it's harmless" so
> > there seems little appetite for an LWG issue to revisit the subject.
> >
> > This patch changes the libstdc++ implementation to be in agreement with
> > the standard by removing const from those notify_one/notify_all members.
> >
> > libstdc++-v3/ChangeLog:
>
> Might as well add a "PR libstdc++/102994" here to the bug gets updated
> automatically.
>
> OK for trunk with that change.
>
> > * include/bits/atomic_base.h (atomic_flag::notify_one,
> > notify_all): Remove const qualification.
> > (__atomic_base::notify_one, notify_all): Likewise.
> > * include/std/atomic (atomic<bool>::notify_one, notify_all):
> > Likewise.
> > (atomic::notify_one, notify_all): Likewise.
> > (atomic<T*>::notify_one, notify_all): Likewise.
> > (atomic_notify_one, atomic_notify_all): Likewise.
> > * testsuite/29_atomics/atomic/wait_notify/102994.cc: Adjust test
> > to account for change in notify_one/notify_all signature.
> >
> > Tested x86_64-pc-linux-gnu.
>
>
From 7ed6dfae5a0a7a9e56291d780e44f99d644847e0 Mon Sep 17 00:00:00 2001
From: Thomas Rodgers <rodgert@appliantology.com>
Date: Thu, 10 Feb 2022 18:55:16 -0800
Subject: [PATCH] libstdc++: Make atomic notify_one and notify_all non-const
<recording this here for future reference>
PR102994 "atomics: std::atomic<ptr>::wait is not marked const" raises the
issue that the current libstdc++ implementation marks the notify members
const, the implementation strategy used by libstdc++, as well as libc++
and the Microsoft STL, do not require the atomic to be mutable (it is hard
to conceive of a desirable implementation approach that would require it).
The original paper proposing the wait/notify functionality for atomics
(p1185) also had these members marked const for the first three revisions,
but that was changed without explanation in r3 and subsequent revisions of
the paper.
After raising the issue to the authors of p1185 and the author of the
libc++ implementation, the consensus seems to be "meh, it's harmless" so
there seems little appetite for an LWG issue to revisit the subject.
This patch changes the libstdc++ implementation to be in agreement with
the standard by removing const from those notify_one/notify_all members.
libstdc++-v3/ChangeLog:
* include/bits/atomic_base.h (atomic_flag::notify_one,
notify_all): Remove const qualification.
(__atomic_base::notify_one, notify_all): Likewise.
* include/std/atomic (atomic<bool>::notify_one, notify_all):
Likewise.
(atomic::notify_one, notify_all): Likewise.
(atomic<T*>::notify_one, notify_all): Likewise.
(atomic_notify_one, atomic_notify_all): Likewise.
* testsuite/29_atomics/atomic/wait_notify/102994.cc: Adjust test
to account for change in notify_one/notify_all signature.
---
libstdc++-v3/include/bits/atomic_base.h | 8 ++++----
libstdc++-v3/include/std/atomic | 16 ++++++++--------
.../29_atomics/atomic/wait_notify/102994.cc | 4 ++--
3 files changed, 14 insertions(+), 14 deletions(-)
@@ -252,13 +252,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// TODO add const volatile overload
_GLIBCXX_ALWAYS_INLINE void
- notify_one() const noexcept
+ notify_one() noexcept
{ std::__atomic_notify_address(&_M_i, false); }
// TODO add const volatile overload
_GLIBCXX_ALWAYS_INLINE void
- notify_all() const noexcept
+ notify_all() noexcept
{ std::__atomic_notify_address(&_M_i, true); }
// TODO add const volatile overload
@@ -600,13 +600,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// TODO add const volatile overload
_GLIBCXX_ALWAYS_INLINE void
- notify_one() const noexcept
+ notify_one() noexcept
{ std::__atomic_notify_address(&_M_i, false); }
// TODO add const volatile overload
_GLIBCXX_ALWAYS_INLINE void
- notify_all() const noexcept
+ notify_all() noexcept
{ std::__atomic_notify_address(&_M_i, true); }
// TODO add const volatile overload
@@ -172,11 +172,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// TODO add const volatile overload
void
- notify_one() const noexcept
+ notify_one() noexcept
{ _M_base.notify_one(); }
void
- notify_all() const noexcept
+ notify_all() noexcept
{ _M_base.notify_all(); }
#endif // __cpp_lib_atomic_wait
};
@@ -399,11 +399,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// TODO add const volatile overload
void
- notify_one() const noexcept
+ notify_one() noexcept
{ std::__atomic_notify_address(&_M_i, false); }
void
- notify_all() const noexcept
+ notify_all() noexcept
{ std::__atomic_notify_address(&_M_i, true); }
#endif // __cpp_lib_atomic_wait
@@ -654,11 +654,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// TODO add const volatile overload
void
- notify_one() const noexcept
+ notify_one() noexcept
{ _M_b.notify_one(); }
void
- notify_all() const noexcept
+ notify_all() noexcept
{ _M_b.notify_all(); }
#endif // __cpp_lib_atomic_wait
@@ -1434,12 +1434,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _Tp>
inline void
- atomic_notify_one(const atomic<_Tp>* __a) noexcept
+ atomic_notify_one(atomic<_Tp>* __a) noexcept
{ __a->notify_one(); }
template<typename _Tp>
inline void
- atomic_notify_all(const atomic<_Tp>* __a) noexcept
+ atomic_notify_all(atomic<_Tp>* __a) noexcept
{ __a->notify_all(); }
#endif // __cpp_lib_atomic_wait
@@ -5,13 +5,13 @@
#include <atomic>
void
-test1(const std::atomic<char*>& a, char* p)
+test1(std::atomic<char*>& a, char* p)
{
a.wait(p);
}
void
-test2(const std::atomic<int>* a, int v)
+test2(std::atomic<int>* a, int v)
{
std::atomic_wait(a, v);
std::atomic_notify_one(a);
--
2.34.1