[libcxx] Make stdatomic.h work when included from a C source file

[ngg]

Summary

If a C source file includes the libc++ stdatomic.h, compilation will
break because (a) the C++ standard check will fail (which is expected),
and (b) _LIBCPP_COMPILER_CLANG_BASED won't be defined because the
logic defining it in __config is guarded by a __cplusplus check, so
we'll end up with a blank header. Move the detection logic outside of
the __cplusplus check to make the second check pass even in a C context
when you're using Clang. Note that _LIBCPP_STD_VER is not defined when
in C mode, hence stdatomic.h needs to check if in C++ mode before using
that macro to avoid a warning.

In an ideal world, a C source file wouldn't be including the libc++
header directory in its search path, so we'd never have this issue.
Unfortunately, certain build environments make this hard to guarantee,
and in this case it's easy to tweak this header to make it work in a C
context, so I'm hoping this is acceptable.

Fixes https://github.com/llvm/llvm-project/issues/57710.

Note: I couldn't update the https://reviews.llvm.org/D131435 revision, that's why I've created a new one.

[ldionne]

Thanks for picking this up. I think you probably needed to "Commandeer" D131435 in order to update it. But this will do, I'll ask @smeenai to close D131435.

Let's try to land this quickly, I realize only now that this is the source of some failures I've been seeing internally as well, so this is likely hitting a bunch of people and we should cherry-pick back to LLVM 15.

libcxx/include/stdatomic.h

124

Why is this change required?

[ngg]

libcxx/include/stdatomic.h

124

I've tried to describe it in the commit message: "Note that _LIBCPP_STD_VER is not defined when in C mode, hence stdatomic.h needs to check if in C++ mode before using that macro to avoid a warning."

_LIBCPP_STD_VER is defined only when in C++ mode, this triggers a warning "treating undefined as 0", and the include_as_c.sh.cpp test even fails because it uses -Werror

[ldionne]

libcxx/include/stdatomic.h

124

Oh, of course, this makes sense. Thanks and sorry for the dumbish question.

[ngg]

@ldionne can you please merge this? I don't have push permissions. Thanks!

[ldionne]

In D134591#3817045, @ngg wrote:

@ldionne can you please merge this? I don't have push permissions. Thanks!

Sure, can you please provide Author Name <email@domain> for attribution please?

[ngg]

In D134591#3817791, @ldionne wrote:

Sure, can you please provide Author Name <email@domain> for attribution please?

Gergely Nagy <[e-mail]>

libcxx/include/__config

 #    define _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER
 #  endif
 #endif
 
 #ifndef _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER
 #  pragma GCC system_header
 #endif
 
+#if defined(__apple_build_version__)
+#  define _LIBCPP_COMPILER_CLANG_BASED
+#  define _LIBCPP_APPLE_CLANG_VER (__apple_build_version__ / 10000)
+#elif defined(__clang__)
+#  define _LIBCPP_COMPILER_CLANG_BASED
+#  define _LIBCPP_CLANG_VER (__clang_major__ * 100 + __clang_minor__)
+#elif defined(__GNUC__)
+#  define _LIBCPP_COMPILER_GCC
+#endif
+
 #ifdef __cplusplus
 
 // _LIBCPP_VERSION represents the version of libc++, which matches the version of LLVM.
 // Given a LLVM release LLVM XX.YY.ZZ (e.g. LLVM 16.0.1 == 16.00.01), _LIBCPP_VERSION is
 // defined to XXYYZZ.
 #  define _LIBCPP_VERSION 160000
 
 #  define _LIBCPP_CONCAT_IMPL(_X, _Y) _X##_Y

 #  endif
 
 #  define __has_keyword(__x) !(__is_identifier(__x))
 
 #  ifndef __has_include
 #    define __has_include(...) 0
 #  endif
 
-#  if defined(__apple_build_version__)
-#    define _LIBCPP_COMPILER_CLANG_BASED
-#    define _LIBCPP_APPLE_CLANG_VER (__apple_build_version__ / 10000)
-#  elif defined(__clang__)
-#    define _LIBCPP_COMPILER_CLANG_BASED
-#    define _LIBCPP_CLANG_VER (__clang_major__ * 100 + __clang_minor__)
-#  elif defined(__GNUC__)
-#    define _LIBCPP_COMPILER_GCC
-#  endif
-
 #  if !defined(_LIBCPP_COMPILER_CLANG_BASED) && __cplusplus < 201103L
 #    error "libc++ only supports C++03 with Clang-based compilers. Please enable C++11"
 #  endif
 
 // FIXME: ABI detection should be done via compiler builtin macros. This
 // is just a placeholder until Clang implements such macros. For now assume
 // that Windows compilers pretending to be MSVC++ target the Microsoft ABI,
 // and allow the user to explicitly specify the ABI to handle cases where this

libcxx/include/stdatomic.h

 */
 
 #include <__config>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
-#if _LIBCPP_STD_VER > 20
+#if defined(__cplusplus) && _LIBCPP_STD_VER > 20
 
 #include <atomic>
 #include <version>
 
 #ifdef _Atomic
 # undef _Atomic
 #endif
 

 // Before C++23, we include the next <stdatomic.h> on the path to avoid hijacking
 // the header. We do this because Clang has historically shipped a <stdatomic.h>
 // header that would be available in all Standard modes, and we don't want to
 // break that use case.
 # if __has_include_next(<stdatomic.h>)
 #   include_next <stdatomic.h>
 # endif
 
-#endif // _LIBCPP_STD_VER > 20
+#endif // defined(__cplusplus) && _LIBCPP_STD_VER > 20
 
 #endif // _LIBCPP_STDATOMIC_H

libcxx/test/libcxx/include_as_c.sh.cpp

 #include <float.h>
 #include <inttypes.h>
 #include <limits.h>
 #ifndef _LIBCPP_HAS_NO_LOCALIZATION
 #   include <locale.h>
 #endif
 #include <math.h>
 #include <setjmp.h>
+#include <stdatomic.h>
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 // The clang-shipped tgmath.h header doesn't work with MSVC/UCRT's complex
 // headers in C mode, see PR46207.