Undef stdatomic.h macro definitions that are defining functions provided in libc++ <atomic>

[mehdi_amini]

Summary

This solved compiling:

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

[mehdi_amini]

Ping @rsmith !

[rsmith]

Hmm, this won't help when building libc++ as a module, and we don't have a wrapper header to hold these undefs since <stdatomic.h> is not part of c++.

So either that combination of includes gives a broken <atomic> or a broken <stdatomic.h>, or we do something nonstandard like reimplementing the latter in terms of the former in libc++.

Eric, Marshall, what do you think? How / how much should we support this non-c++ header?

[mclow.lists]

I'll do some research.

[mclow.lists]

Ok - looking just at kill_dependency, this was added to the C standard in C11. It's required to be a macro.
atomic_is_lock_free appears to be a function, as does atomic_load. Haven't checked the rest.

I see that Bionic (sometimes) defines atomic_is_lock_free as a macro.

[mclow.lists]

More info - The following code:

#include <stdatomic.h>
int main () {}

fails to compile on either gcc 6.2 (locally), gcc 7 head (online compiler) or MSVC (online compiler).

[EricWF]

Yeah this seems like a configuration that simply can't be supported. I'm not sure if this patch is a great idea.

[mehdi_amini]

In D26376#597614, @mclow.lists wrote:

More info - The following code:

#include <stdatomic.h>
int main () {}

fails to compile on either gcc 6.2 (locally), gcc 7 head (online compiler) or MSVC (online compiler).

Interesting, that lead me to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60932 which describes the issue I believe.

One of the example is that we should guarantee ABI compatibility between C and C++ for such code:

#ifdef __cplusplus
#include <atomic>
using namespace std;
#else
#include <stdatomic.h>
#endif

struct s {
  atomic_int i;
};

Do you know if we're providing this guarantee today?

In D26376#613186, @EricWF wrote:

Yeah this seems like a configuration that simply can't be supported. I'm not sure if this patch is a great idea.

OK, let's abandon this then!

[mehdi_amini]

Thanks for the information! Closing this.

[bumblebritches57]

This is the wrong approach.

C and C++ compatibility is far more important than taking the easy way out.

By doing this, you're potentially breaking the ABI for all software that relies on atomic operations...

libcxx/include/atomic

 #if !defined(_LIBCPP_HAS_C_ATOMIC_IMP) && !defined(_LIBCPP_HAS_GCC_ATOMIC_IMP)
 #error <atomic> is not implemented
 #endif
 
 #if _LIBCPP_STD_VER > 14
 # define __cpp_lib_atomic_is_always_lock_free 201603L
 #endif
 
+// stdatomic.h may implement these as a macros, be safe and undef.
+#ifdef kill_dependency
+#undef kill_dependency
+#endif
+#ifdef atomic_is_lock_free
+#undef atomic_is_lock_free
+#endif
+#ifdef atomic_load
+#undef atomic_load
+#endif
+#ifdef atomic_exchange
+#undef atomic_exchange
+#endif
+#ifdef atomic_compare_exchange_weak
+#undef atomic_compare_exchange_weak
+#endif
+#ifdef atomic_compare_exchange_strong
+#undef atomic_compare_exchange_strong
+#endif
+#ifdef atomic_fetch_and
+#undef atomic_fetch_and
+#endif
+#ifdef atomic_fetch_add
+#undef atomic_fetch_add
+#endif
+#ifdef atomic_fetch_sub
+#undef atomic_fetch_sub
+#endif
+#ifdef atomic_store
+#undef atomic_store
+#endif
+#ifdef atomic_store_explicit
+#undef atomic_store_explicit
+#endif
+#ifdef atomic_flag_test_and_set
+#undef atomic_flag_test_and_set
+#endif
+#ifdef atomic_flag_test_and_set_explicit
+#undef atomic_flag_test_and_set_explicit
+#endif
+#ifdef atomic_exchange_explicit
+#undef atomic_exchange_explicit
+#endif
+#ifdef atomic_flag_clear
+#undef atomic_flag_clear
+#endif
+#ifdef atomic_fetch_xor
+#undef atomic_fetch_xor
+#endif
+#ifdef atomic_flag_clear_explicit
+#undef atomic_flag_clear_explicit
+#endif
+#ifdef atomic_fetch_or
+#undef atomic_fetch_or
+#endif
+
+
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 typedef enum memory_order
 {
     memory_order_relaxed, memory_order_consume, memory_order_acquire,
     memory_order_release, memory_order_acq_rel, memory_order_seq_cst
 } memory_order;