[libcxx-commits] [libcxx] [libc++][Android] Always redirect <stdatomic.h> to <atomic> (PR #143036)

[Aaron Ballman via libcxx-commits]

AaronBallman wrote:

> I spoke with Nikolas about this issue just now and here's another possibility we came up with (which is a variation on option 1 above):

FWIW, I think the bar to clear for Option 1 is quite high.

https://sourcegraph.com/search?q=context:global+file:.*%5C.cpp+-file:.*test.*+%5Cb_Atomic%5Cb+count:1000000&patternType=regexp&case=yes&sm=0

That's searching just files with a .cpp extension for use of `_Atomic` and coming up with 8k+ uses in the wild; there are a lot more uses when you start considering header files (but that's harder to count precisely because the header could be a C++ header or it could be a C header included in a C++ compilation unit). Repeating the search but setting the language to C++ (and dropping the file extension) gives 76k+ results but I'm pretty sure there's plenty of false positives in that count. But I think 30-50k uses is a reasonable guesstimate, which shows it is a pretty popular extension. (By comparison, we also support `_Thread_local` in C++ and those searches come up with 1k and 10k, respectively)

>     1. Introduce a new Clang "keyword" for `_Atomic` (same functionality, new spelling). Let's call it `_AtomicExt` for now.
> 
>     2. Deprecate the existing `_Atomic` extension in Clang. After N releases, require that users include `<stdatomic.h>` before using `_Atomic` (in all C++ modes/versions). Failure to do so is an error. Using `_AtomicExt` without including `<stdatomic.h>` is fine, though.
> 
>     3. Libc++ always provides `<stdatomic.h>`, which defines `_Atomic(T)` to be `std::atomic<T>`. Under the hood, libc++ keeps implementing `std::atomic` using the Clang extension (now named `_AtomicExt`).

This is the usual deprecation-for-removal approach, so it's not novel. But this extension has been supported in Clang since Clang 3.1 (https://godbolt.org/z/8e7z6rMc7) which was released May 2012. It's also an explicitly documented extension: https://clang.llvm.org/docs/LanguageExtensions.html#language-extensions-back-ported-to-previous-standards

> Pros:
> 
>     * Solves the ODR/ABI issues that exist with the status quo across standard modes, **but also with whether you include `<stdatomic.h>` or not**
> 
>     * Does not require changing the C++ Standard
> 
>     * Android gets the behavior they currently have, upstream
> 
>     * There's still a way to access the original Clang extension by using the `_AtomicExt` name (potentially needed by users and needed by libc++ itself).
> 
> 
> Cons:
> 
>     * Changes the meaning of `_Atomic` on non-Android platforms, which is a breaking change. But that's handled via the migration path and it's not a silent change.
> 
>     * `_Atomic(T)` means slightly different things between C and C++, but that's the route that C++23 already decided to go down. We can also probably make small changes to make both more ABI-compatible (for example https://godbolt.org/z/o53Pbffe5).
> 
> 
> I think this is the path we both like the most so far. @AaronBallman do you have an opinion about the above? This means a bit of work, but we would end up with a much nicer story for bridging C and C++ atomics, and more consistency within our C++ support.

I think it's worth exploring the idea but my initial inclination is not positive It's not that it's a bad idea per se, but I think it erodes trust with our users given that we've supported this extensions for about a decade and a half and we document it as an extension we support. It'd be different if the extension wasn't documented or used much in the wild, but once we document it and users start to use it, deprecation and replacement is a pretty expensive option.

https://github.com/llvm/llvm-project/pull/143036