[Bug libstdc++/102994] New: std::atomic<ptr>::wait is not marked const

[Bug libstdc++/102994] New: std::atomic<ptr>::wait is not marked const

[gcc_bugzilla at axeitado dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

            Bug ID: 102994
           Summary: std::atomic<ptr>::wait is not marked const
           Product: gcc
           Version: 11.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gcc_bugzilla at axeitado dot com
  Target Milestone: ---

This code fails to compile because the `wait' member function is not marked
`const' in the specialization of std::atomic for pointers:

#include <atomic>

void foo(const std::atomic<char*> &a, char *p) {
    a.wait(p);
}


g++ --std=c++20

<source>: In function 'void foo(const std::atomic<char*>&, char*)':
<source>:4:11: error: passing 'const std::atomic<char*>' as 'this' argument
discards qualifiers [-fpermissive]
    4 |     a.wait(p);
      |     ~~~~~~^~~
In file included from <source>:1:
/opt/compiler-explorer/gcc-trunk-20211028/include/c++/12.0.0/atomic:651:5:
note:   in call to 'void
std::atomic<_Tp*>::wait(std::atomic<_Tp*>::__pointer_type, std::memory_order)
[with _Tp = char; std::atomic<_Tp*>::__pointer_type = char*]'
  651 |     wait(__pointer_type __old, memory_order __m = memory_order_seq_cst)
noexcept
      |     ^~~~
Compiler returned: 1

[Bug libstdc++/102994] std::atomic<ptr>::wait is not marked const

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

Jonathan Wakely <redi at gcc dot gnu.org> 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

[Bug libstdc++/102994] std::atomic<ptr>::wait is not marked const

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

--- Comment #1 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Thomas Rodgers <rodgertq@gcc.gnu.org>:

https://gcc.gnu.org/g:38c60e5075f89265a560eab166d43247624a7535

commit r12-5886-g38c60e5075f89265a560eab166d43247624a7535
Author: Thomas Rodgers <rodgert@twrodgers.com>
Date:   Thu Dec 9 15:35:25 2021 -0800

    libstdc++: Make atomic<T*>::wait() const [PR102994]

    This was an oversight in the original commit adding wait/notify
    to atomic<T>.

    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<ptr>::wait is not marked const

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Thomas Rodgers
<rodgertq@gcc.gnu.org>:

https://gcc.gnu.org/g:aca7d4e8790bff32ee749936bb0224d873c46052

commit r11-9371-gaca7d4e8790bff32ee749936bb0224d873c46052
Author: Thomas Rodgers <rodgert@twrodgers.com>
Date:   Thu Dec 9 15:35:25 2021 -0800

    libstdc++: Make atomic<T*>::wait() const [PR102994]

    This was an oversight in the original commit adding wait/notify
    to atomic<T>.

    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<ptr>::wait is not marked const

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |11.3

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for 11.3

[Bug libstdc++/102994] std::atomic<ptr>::wait is not marked const

[gcc_bugzilla at axeitado dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

Óscar Fuentes <gcc_bugzilla at axeitado dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

--- Comment #4 from Óscar Fuentes <gcc_bugzilla at axeitado dot com> ---
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<ptr>::wait is not marked const

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(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<ptr>::wait is not marked const

[gcc_bugzilla at axeitado dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

--- Comment #6 from Óscar Fuentes <gcc_bugzilla at axeitado dot com> ---
(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 <X.xx again in the future if the change is not accepted
and is reverted.

[Bug libstdc++/102994] std::atomic<ptr>::wait is not marked const

[rodgertq at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

--- Comment #7 from Thomas Rodgers <rodgertq at gcc dot gnu.org> ---
(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 <X.xx again in the future if the change is
> 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<ptr>::wait is not marked const

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
*** Bug 103933 has been marked as a duplicate of this bug. ***

[Bug libstdc++/102994] std::atomic<ptr>::wait is not marked const

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(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 <X.xx again in the future if the change is
> 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<ptr>::wait is not marked const

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
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<T>::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<ptr>::wait is not marked const

[rodgertq at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

Thomas Rodgers <rodgertq at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |SUSPENDED

--- Comment #11 from Thomas Rodgers <rodgertq at gcc dot gnu.org> ---
(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<T>::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<ptr>::wait is not marked const

[gcc_bugzilla at axeitado dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

Óscar Fuentes <gcc_bugzilla at axeitado dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|SUSPENDED                   |RESOLVED
         Resolution|---                         |FIXED

--- Comment #12 from Óscar Fuentes <gcc_bugzilla at axeitado dot com> ---
(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 <X.xx again in the future if the change is
> > 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<int>*), std::atomic_notify_all);

The "_OV" means "overloaded", "void" is the type returned, (std::atomic<int>*)
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<std::atomic<int>>);

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<ptr>::wait is not marked const

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |SUSPENDED
         Resolution|FIXED                       |---

--- Comment #13 from Jonathan Wakely <redi at gcc dot gnu.org> ---
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<ptr>::wait is not marked const

[rodgertq at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

--- Comment #14 from Thomas Rodgers <rodgertq at gcc dot gnu.org> ---
Created attachment 52420
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52420&action=edit
Make notify_one/notify_all non-const

I submitted this patch to the list.

[Bug libstdc++/102994] std::atomic<ptr>::wait is not marked const

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|SUSPENDED                   |ASSIGNED

[Bug libstdc++/102994] std::atomic<ptr>::wait is not marked const

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.3                        |11.4

--- Comment #15 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 11.3 is being released, retargeting bugs to GCC 11.4.

[Bug libstdc++/102994] std::atomic<ptr>::wait is not marked const

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

--- Comment #16 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Thomas Rodgers <rodgertq@gcc.gnu.org>:

https://gcc.gnu.org/g:7c21556daf385fe9ece37319f574776dd7d8ab1c

commit r12-8231-g7c21556daf385fe9ece37319f574776dd7d8ab1c
Author: Thomas W Rodgers <trodgers@redhat.com>
Date:   Fri Apr 22 15:46:19 2022 -0700

    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:

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

[Bug libstdc++/102994] std::atomic<ptr>::wait is not marked const

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

--- Comment #17 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Thomas Rodgers
<rodgertq@gcc.gnu.org>:

https://gcc.gnu.org/g:977cbabeb1c1a9a28cc45655bdc693e1594642f0

commit r11-9931-g977cbabeb1c1a9a28cc45655bdc693e1594642f0
Author: Thomas W Rodgers <trodgers@redhat.com>
Date:   Fri Apr 22 15:46:19 2022 -0700

    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:

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

    (cherry picked from commit 7c21556daf385fe9ece37319f574776dd7d8ab1c)

[Bug libstdc++/102994] std::atomic<ptr>::wait is not marked const

[rodgertq at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102994

Thomas Rodgers <rodgertq at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #18 from Thomas Rodgers <rodgertq at gcc dot gnu.org> ---
Committed to GCC12 and backported to GCC11