[BUG] Compilation error using _Atomic type in C++ (r21 regression)

[triplef]

Description

Using the C-style _Atomic type in a C++ file fails to compile with NDK r21, but compiles fine with r20 and on other platforms.

test.cpp:

#include <stdatomic.h>

void test() {
    _Atomic int p;
    atomic_load_explicit(&p, memory_order_relaxed);
}

Error:

test.cpp:5:2: error: no matching function for call to 'atomic_load_explicit'
        atomic_load_explicit(&p, memory_order_relaxed);
        ^~~~~~~~~~~~~~~~~~~~
/Users/me/Library/Android/sdk/ndk/21.0.6113669/toolchains/llvm/prebuilt/darwin-x86_64/sysroot/usr/include/c++/v1/atomic:1816:1: note: candidate template ignored: could not match 'atomic<type-parameter-0-0>' against '_Atomic(int) *'
atomic_load_explicit(const volatile atomic<_Tp>* __o, memory_order __m) _NOEXCEPT
^
/Users/me/Library/Android/sdk/ndk/21.0.6113669/toolchains/llvm/prebuilt/darwin-x86_64/sysroot/usr/include/c++/v1/atomic:1825:1: note: candidate template ignored: could not match 'atomic<type-parameter-0-0>' against '_Atomic(int) *'
atomic_load_explicit(const atomic<_Tp>* __o, memory_order __m) _NOEXCEPT
^

I think the issue is caused by the same change in bionic as #1177.

This e.g. causes the swift-corelibs-libdispatch project to fail to compile with r21 due to its use of atomic types in atomic.h, which is used in C++.

Environment Details

• NDK Version: 21.0.6113669
• Build system: manually running clang++
• Host OS: macOS
• ABI: any
• NDK API level: any

[alexcohn]

Note that you code does not compile on NDK r20:

test.cpp:5:5: error: address argument to atomic operation must be a pointer to _Atomic type ('_Atomic(int) **' invalid)
      atomic_load_explicit(&p, memory_order_relaxed);
      ^                    ~~

That's a simple copy/paste typo, line 4 should be

    _Atomic int p;

[triplef]

Ah yes, thank you. I’ve fixed the code above (I had initially been using a void pointer...).

[alexcohn]

The easy fix is to use <bits/stdatomics.h>. Instead of modifying the code, you can put …/lib64/clang/9.0.8/include/bits early on your isystem list. See #1178 (comment)!

For me, it's enough to add to build.gradle:

cppFlags "-isystem $(NDK_ROOT)\\toolchains\\llvm\\prebuilt\\windows-x86_64\\lib64\\clang\\9.0.8\\include\\bits"

[triplef]

Yes, including <bits/stdatomics.h> works fine, and it’s what I did to work around this issue and #1177.

However I’d rather not have to change the code or hardcode llvm paths in the build script, so it would be great to see this fixed in a future NDK release. 👍

[DanAlbert]

I would strongly recommend against adding the bits directory to your include path. This might work to fix stdatomic.h for this release, but it could quite easily give you the wrong header in other cases.

[alexcohn]

@DanAlbert, you are right.

I believe that the basic idea of using <atomic> inside <stdatomic.h> is flawed. People who include the latter expect it to work C11-style, not as a wrapper around STL.

[DanAlbert]

We'll be fixing the regression in r21b. Thanks for being clear up front that this was a regression from r20. That makes triage easy :)

I believe that the basic idea of using inside <stdatomic.h> is flawed. People who include the latter expect it to work C11-style, not as a wrapper around STL.

tl;dr wrapping <atomic> in <stdatomic.h> is intended to get you C11 style

C++ atomics were designed to be compatible with C atomics, but the standard specified behavior is currently broken. You can't have <stdatomic.h> and <atomic> in the same translation unit, let alone source file. C's <stdatomic.h> defines as macros the same names that are in <atomic> as template functions. When you reach <atomic>, the preprocessor performs macro replacement of the names in the function declarations, and the whole thing falls apart.

The expectation is that http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0943r4.html will be the fix for this. This proposal describes the behavior we've had in AOSP for the past ~6 years. Using <stdatomic.h> in C++ will actually get you <atomic>, but with names in the global namespace. That's compatible with C11, so the same headers will work with either C11 or C++23 (libc++ will probably backport the behavior as well).

Note that the behavior you're describing has precedent in C++ already. <complex.h> is defined to #include <ccomplex> (into the global namespace) which is defined to #include <complex>.

[triplef]

That’s great news @DanAlbert! Can we expect r21b to also fix #1177?

[alexcohn]

When you reach <atomic>, the preprocessor performs macro replacement of the names in the function declarations, and the whole thing falls apart.

If I am not missing something, I see something exactly opposite:

#include <atomic>
extern "C" {
#include <stdatomic.h>
}

passes compilation.

[DanAlbert]

Other way around.

#include <stdatomic.h>
#include <atomic>

...
[arm64-v8a] Compile++      : foo <= foo.cpp
In file included from jni/foo.cpp:2:
/work/src/android-ndk-r20/sources/cxx-stl/llvm-libc++/include/atomic:1166:49: error: expected ')'
atomic_is_lock_free(const volatile atomic<_Tp>* __o) _NOEXCEPT
                                                ^
/work/src/android-ndk-r20/sources/cxx-stl/llvm-libc++/include/atomic:1166:1: note: to match this '('
atomic_is_lock_free(const volatile atomic<_Tp>* __o) _NOEXCEPT
^
/work/src/android-ndk-r20/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/8.0.7/include/stdatomic.h:250:68: note: expanded from macro 'atomic_is_lock_free'
#define atomic_is_lock_free(obj) __c11_atomic_is_lock_free(sizeof(*(obj)))
                                                                   ^
In file included from jni/foo.cpp:2:
/work/src/android-ndk-r20/sources/cxx-stl/llvm-libc++/include/atomic:1166:1: error: expected expression
atomic_is_lock_free(const volatile atomic<_Tp>* __o) _NOEXCEPT
^
...

[DanAlbert]

@triplef Yep. Sorry, been bad at triage this week. Fixed that now.

[alexcohn]

Other way around.

This fails with r20, but passes with r21.

[smeenai]

Something worth noting is that libdispatch has its own macro wrappers around _Atomic (e.g. https://github.com/apple/swift-corelibs-libdispatch/blob/c992dacf3ca114806e6ac9ffc9113b19255be9fe/src/shims/atomic.h#L46), so you'll want to confirm that whatever solution is used for r21b works with that case too.