Proposal: Allow std::atomic_ref

[Peter Kasting]

C++20 adds std::atomic_ref, which allows safe atomic access to storage that is non-atomic at low levels. This is a necessary use case various places (e.g. V8), and the only solution before this is a reinterpret_cast, which is technically UB, although the clang folks formally blessed it.

We didn't previously allow this because it wasn't implemented in libc++. With https://github.com/llvm/llvm-project/issues/82369 fixed, that is no longer true. Therefore I propose we allow.

I don't know whether there are still known sharp edges/bugs/missing features. I would appreciate explicit input from any Lexan folks on the safety here.

PK

[Hans Wennborg]

I'm not aware of any issues with std::atomic_ref. 

[Joe Mason]

I coincidentally just learned about std::atomic_ref. +1 to allowing it now that the library support is there.

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAB8jPhd1FNBybiMEaisxLASMRJ%2BOJ-oEj4NcWwvzrxPTGK3P8Q%40mail.gmail.com.

[Roland McGrath]

There are some ongoing unresolved issues with using qualified types in the parameter to std::atomic_ref, e.g. std::atomic_ref<const T> or std::atomic_ref<volatile T>.  If those are avoided then libc++ std::atomic_ref is ready for prime time.

[Peter Kasting]

On Tue, Aug 20, 2024 at 11:28 AM Roland McGrath <mcgr...@chromium.org> wrote:

There are some ongoing unresolved issues with using qualified types in the parameter to std::atomic_ref, e.g. std::atomic_ref<const T> or std::atomic_ref<volatile T>.  If those are avoided then libc++ std::atomic_ref is ready for prime time.

What sorts of issues, and is there a rough resolution timeline? Do we need to ban cv qualifiers in the template param for now?

PK

[Roland McGrath]

https://github.com/llvm/llvm-project/issues/98689

https://github.com/llvm/llvm-project/issues/94879

https://isocpp.org/files/papers/P3323R0.html 

I would recommend banning qualifiers in the parameter to std::atomic_ref for the foreseeable future. 

[Peter Kasting]

Sounds like the summary is "the standard currently allows these, but they're underspecified; paper poses banning some cases and better-specifying others. Until forward direction is clear, libc++ will just continue to compile-error".

I commented on https://github.com/llvm/llvm-project/issues/94879 hoping for quicker support of the sane cases. In the meantime, though, I think we can just write an explanatory note in the features doc -- "desirable thing doesn't compile" is annoying-but-safe. "Compiles but is subtly broken" would be much scarier.

PK

[Peter Kasting]

It sounds like consensus is to allow, with a note that some desirable cases (cv-qualified `T`) won't compile pending implementation in libc++, which in turn is pending resolution of https://isocpp.org/files/papers/P3323R0.html; in such cases, continuing to `reinterpret_cast<>` is technically UB but formally blessed as the current workaround.

I am checking with gbiv@ whether libc++ is part of the CrOS LLVM toolchain roll; if so, this commit hasn't landed in CrOS yet and we can't allow until it does. If not, or else after that point, I consider this resolved modulo contrary feedback, and I'll look into allowing/migrating in the future.

PK

[Peter Kasting]

On Wed, Sep 4, 2024 at 9:16 AM Peter Kasting <pkas...@chromium.org> wrote:

I am checking with gbiv@ whether libc++ is part of the CrOS LLVM toolchain roll

Answer: no. I filed https://issuetracker.google.com/364618746 to try and ensure we get this in the next roll. Timeframe is unknown, but a reasonable estimate would be Nov/Dec unless we decide this is urgent (IMO, no).

PK