[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 Thomas Rodgers changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #18 from Thomas Rodgers --- Committed to GCC12 and backported to GCC11
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 --- Comment #17 from CVS Commits --- The releases/gcc-11 branch has been updated by Thomas Rodgers : https://gcc.gnu.org/g:977cbabeb1c1a9a28cc45655bdc693e1594642f0 commit r11-9931-g977cbabeb1c1a9a28cc45655bdc693e1594642f0 Author: Thomas W Rodgers Date: Fri Apr 22 15:46:19 2022 -0700 libstdc++: Make atomic notify_one and notify_all non-const PR102994 "atomics: std::atomic::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: PR libstdc++/102994 * 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::notify_one, notify_all): Likewise. (atomic::notify_one, notify_all): Likewise. (atomic::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. (cherry picked from commit 7c21556daf385fe9ece37319f574776dd7d8ab1c)
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 --- Comment #16 from CVS Commits --- The master branch has been updated by Thomas Rodgers : https://gcc.gnu.org/g:7c21556daf385fe9ece37319f574776dd7d8ab1c commit r12-8231-g7c21556daf385fe9ece37319f574776dd7d8ab1c Author: Thomas W Rodgers Date: Fri Apr 22 15:46:19 2022 -0700 libstdc++: Make atomic notify_one and notify_all non-const PR102994 "atomics: std::atomic::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: PR libstdc++/102994 * 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::notify_one, notify_all): Likewise. (atomic::notify_one, notify_all): Likewise. (atomic::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.
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 Richard Biener changed: What|Removed |Added Target Milestone|11.3|11.4 --- Comment #15 from Richard Biener --- GCC 11.3 is being released, retargeting bugs to GCC 11.4.
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 Jonathan Wakely changed: What|Removed |Added Status|SUSPENDED |ASSIGNED
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 --- Comment #14 from Thomas Rodgers --- Created attachment 52420 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52420=edit Make notify_one/notify_all non-const I submitted this patch to the list.
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 Jonathan Wakely changed: What|Removed |Added Status|RESOLVED|SUSPENDED Resolution|FIXED |--- --- Comment #13 from Jonathan Wakely --- Please don't close it. It was already explained it's suspended pending a response from the committee. Either libstdc++ will change or the standard will change, you just have to be a bit more patient.
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994
Óscar Fuentes changed:
What|Removed |Added
Status|SUSPENDED |RESOLVED
Resolution|--- |FIXED
--- Comment #12 from Óscar Fuentes ---
(In reply to Jonathan Wakely from comment #9)
> (In reply to Óscar Fuentes from comment #6)
> > So IIUC you are applying modifications to libstdc++ that deviate from the
> > published standard expecting that the committee will accept those changes.
> > As a user, this is troublesome, because right now I need to special-case gcc
> > version >11.2 and maybe version > not accepted and is reverted.
>
> Why do you need to special case anything? What problem are these extra const
> qualifications causing you?
One project here consists on a compiler for certain strict, statically typed
language that transparently interacts with C++ code bases. We have a mechanism
for inferring the signature of C/C++ functions and automatically create
wrappers for them, using a combination of macros and templates. For instance,
this is how std::atomic_notify_all is reflected:
LP0_FFI_FN_OV("notify-all", void, (std::atomic*), std::atomic_notify_all);
The "_OV" means "overloaded", "void" is the type returned, (std::atomic*)
is the argument list. If the returned type and argument list does not match an
overload of std::atomic_notify_all, the C++ compiler throws an error.
For stdlibc++ we could simply use
LP0_FFI_FN("notify-all", std::atomic_notify_all>);
and let our template machinery deduce the signature of std::atomic_notify_all,
but other implementations (libc++) do provide the "volatile" overload, so we
are forced to explicitly tell the compiler which overload we want.
Thus, if the function's signature differ from one implementation to another, we
need to detect the correct signature and use it on each instantiation of
std::atomic_notify_all et al we reflect.
To make things worse, some distros picked the change and incorporated them to
their gcc 11.2 packages. I'm afraid the only solution is a platform check at
configure time plus the corresponding macro-sprinkling on our C++ sources. A
hairy mess for what otherwise would be something quite simple and clean.
There are other technical inconveniences related to our precise use case that
would be too long to explain here.
Anyway, thanks for explaining the state of affairs. I understand your POV, so
I'm closing this issue.
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 Thomas Rodgers changed: What|Removed |Added Status|REOPENED|SUSPENDED --- Comment #11 from Thomas Rodgers --- (In reply to Jonathan Wakely from comment #10) > N.B. [member.functions] in the standard says > > "For a non-virtual member function described in the C++ standard library, an > implementation may declare a different set of member function signatures, > provided that any call to the member function that would select an overload > from the set of declarations described in this document behaves as if that > overload were selected." > > In general, being declared with a different signature is permitted. > > Do you have an example where a call to std::atomic::notify_one() that > should be valid according to the standard either fails to compile or > misbehaves, as a result of being const qualified? Pending the outcome of whether there is an LWG issue with the wording, and given this, I am going to mark this issue SUSPENDED.
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 --- Comment #10 from Jonathan Wakely --- N.B. [member.functions] in the standard says "For a non-virtual member function described in the C++ standard library, an implementation may declare a different set of member function signatures, provided that any call to the member function that would select an overload from the set of declarations described in this document behaves as if that overload were selected." In general, being declared with a different signature is permitted. Do you have an example where a call to std::atomic::notify_one() that should be valid according to the standard either fails to compile or misbehaves, as a result of being const qualified?
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 --- Comment #9 from Jonathan Wakely --- (In reply to Óscar Fuentes from comment #6) > So IIUC you are applying modifications to libstdc++ that deviate from the > published standard expecting that the committee will accept those changes. > As a user, this is troublesome, because right now I need to special-case gcc > version >11.2 and maybe version not accepted and is reverted. Why do you need to special case anything? What problem are these extra const qualifications causing you?
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 --- Comment #8 from Jonathan Wakely --- *** Bug 103933 has been marked as a duplicate of this bug. ***
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 --- Comment #7 from Thomas Rodgers --- (In reply to Óscar Fuentes from comment #6) > (In reply to Jonathan Wakely from comment #5) > > (In reply to Óscar Fuentes from comment #4) > > > The fix is wrong. It changes atomic_notify_one and atomic_notify_all > > > instead > > > of atomic<>::wait. > > > > It changed both. > > > > > So right now atomic<>::wait remains unfixed > > > > Are you sure? > > Sigh. Sorry. It would be nice if the commit message mentioned the change to > atomic_notify_* and its motivation, though. > > > > and atomic_notify_(one|all) arg > > > is wrongly marked as const. > > > > This will be the subject of a library issue, potentially fixing the > > standard. The notify functions should be const too. > > So IIUC you are applying modifications to libstdc++ that deviate from the > published standard expecting that the committee will accept those changes. > As a user, this is troublesome, because right now I need to special-case gcc > version >11.2 and maybe version not accepted and is reverted. There is an ongoing discussion between myself and the SG1,LWG, and LEWG chairs (two of which were authors of p1135 which proposes atomic wait/notify) as to whether there is a wording issue with the standard. None of the three major standard library implementations require (as a matter of implementation detail) notify_one/notify_all to be non-const, and indeed the early wording of p1135 had them marked const. Between r2 and r3 of p1135 this was changed, it'cites the minutes of an LEWG discussion as part of the change rationale, but the minutes of that discussion do not give the motivation for the change. One argument is that you would typically wait in a const context and notify in a non-const context, but by that rationale, the constness of atomic_ref::notify is somewhat weird.
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 --- Comment #6 from Óscar Fuentes --- (In reply to Jonathan Wakely from comment #5) > (In reply to Óscar Fuentes from comment #4) > > The fix is wrong. It changes atomic_notify_one and atomic_notify_all instead > > of atomic<>::wait. > > It changed both. > > > So right now atomic<>::wait remains unfixed > > Are you sure? Sigh. Sorry. It would be nice if the commit message mentioned the change to atomic_notify_* and its motivation, though. > > and atomic_notify_(one|all) arg > > is wrongly marked as const. > > This will be the subject of a library issue, potentially fixing the > standard. The notify functions should be const too. So IIUC you are applying modifications to libstdc++ that deviate from the published standard expecting that the committee will accept those changes. As a user, this is troublesome, because right now I need to special-case gcc version >11.2 and maybe version
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 --- Comment #5 from Jonathan Wakely --- (In reply to Óscar Fuentes from comment #4) > The fix is wrong. It changes atomic_notify_one and atomic_notify_all instead > of atomic<>::wait. It changed both. > So right now atomic<>::wait remains unfixed Are you sure? > and atomic_notify_(one|all) arg > is wrongly marked as const. This will be the subject of a library issue, potentially fixing the standard. The notify functions should be const too.
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 Óscar Fuentes changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|FIXED |--- --- Comment #4 from Óscar Fuentes --- The fix is wrong. It changes atomic_notify_one and atomic_notify_all instead of atomic<>::wait. So right now atomic<>::wait remains unfixed and atomic_notify_(one|all) arg is wrongly marked as const.
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 Jonathan Wakely changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED Target Milestone|--- |11.3 --- Comment #3 from Jonathan Wakely --- Fixed for 11.3
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 --- Comment #2 from CVS Commits --- The releases/gcc-11 branch has been updated by Thomas Rodgers : https://gcc.gnu.org/g:aca7d4e8790bff32ee749936bb0224d873c46052 commit r11-9371-gaca7d4e8790bff32ee749936bb0224d873c46052 Author: Thomas Rodgers Date: Thu Dec 9 15:35:25 2021 -0800 libstdc++: Make atomic::wait() const [PR102994] This was an oversight in the original commit adding wait/notify to atomic. libstdc++-v3/ChangeLog: PR libstdc++/102994 * include/bits/atomic_base.h (__atomic_base<_PTp*>::wait()): Add const qualifier. * include/std/atomic (atomic<_Tp*>::wait(), atomic_wait()): Likewise. * testsuite/29_atomics/atomic/wait_notify/102994.cc: New test. (cherry picked from commit 38c60e5075f89265a560eab166d43247624a7535)
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 --- Comment #1 from CVS Commits --- The master branch has been updated by Thomas Rodgers : https://gcc.gnu.org/g:38c60e5075f89265a560eab166d43247624a7535 commit r12-5886-g38c60e5075f89265a560eab166d43247624a7535 Author: Thomas Rodgers Date: Thu Dec 9 15:35:25 2021 -0800 libstdc++: Make atomic::wait() const [PR102994] This was an oversight in the original commit adding wait/notify to atomic. libstdc++-v3/ChangeLog: PR libstdc++/102994 * include/bits/atomic_base.h (__atomic_base<_PTp*>::wait()): Add const qualifier. * include/std/atomic (atomic<_Tp*>::wait(), atomic_wait()): Likewise. * testsuite/29_atomics/atomic/wait_notify/102994.cc: New test.
[Bug libstdc++/102994] std::atomic::wait is not marked const
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994 Jonathan Wakely changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |rodgertq at gcc dot gnu.org Keywords||rejects-valid Ever confirmed|0 |1 Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2021-10-30
