search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
26.81k stars 10.98k forks source link

Regression: stdatomic.h should not forward to MSVC's incompatible version in C11 #59640

Closed davidben closed 1 year ago

davidben commented 1 year ago

This is a regression from e0c3142af075e2ef89395dbed5939071345eb622. (@compnerd FYI)

Originally, Clang's <stdatomic.h> would only forward to the system's copy if __has_include_next(<stdatomic.h>). This meant that, among other things, clang-cl would still provide C11 atomics even though MSVC headers didn't provide it. (This is nice and consistent with clang-cl defining __STDC_VERSION__ >= 201112L without __STDC_NO_ATOMICS__.)

Then C++23 decided to add <stdatomic.h> to improve C11 compatibility. Newer MSVC now provides stdatomic.h, but it only works in C++23. In C, it fails with:

error is not yet supported when compiling as C, but this is planned for a future release.

Ironically, this C11-compatibility improvement in C++23 made clang-cl less C11-compatible. 😄 So, 1ad7de9e92bc2977698e5f6d6493202b50c912d5 made clang's <stdatomic.h> skip the system's if defined(_MSC_VER).

Then ba49d39b20cc5358da28af2ac82bd336028780bc defined the workaround to only skip on MSVC and C. MSVC and C++ would still forward to MSVC's C++23 header. That's still fine and C11-compatible.

However, e0c3142af075e2ef89395dbed5939071345eb622 regressed this. It now targets the workaround to only MSVC and pre-C++23. It seems to have inadvertently dropped the workaround in C11, despite the comment still referencing C. This now breaks clang-cl in C11 mode. clang-cl still defines __STDC_VERSION__ >= 201112L without __STDC_NO_ATOMICS__, yet #include <stdatomic.h> doesn't work.

I think the condition should have been !(defined(_MSC_VER) && (!defined(__cplusplus) || __cplusplus < 202002L)

compnerd commented 1 year ago

Thanks @davidben, I'll take a look at this today.

nico commented 1 year ago

@gz83 please don't leave "fix soon" comments on bug trackers.

gz83 commented 1 year ago

@gz83 please don't leave "fix soon" comments on bug trackers.

Sorry, I have deleted the previous comment

compnerd commented 1 year ago

@davidben I wonder if this is going to be a problem again soon. The latest preview toolset seems to have added C11 support for the atomics as well. But we should fix this to at least allow the header in C mode currently. I think that this was an oversight on @fsb4000 and my side when we converted from __cplusplus-0 < 202002l