[libcxx-commits] [PATCH] D131435: [libcxx] Make stdatomic.h work when included from a C source file

Wed Aug 17 12:44:26 PDT 2022

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

In D131435#3713983 <https://reviews.llvm.org/D131435#3713983>, @smeenai wrote:

> [...]
> and ensuring that was set up properly was the motivation for my question in https://reviews.llvm.org/D121478#inline-1262415)

Sorry, I never noticed that comment. I just replied.

>> I'm not closed to this patch, but I'll request changes just so we can have this discussion.
>
> Like I said above, I have a local fix for this issue, and that works for us, so I think this comes down to whether it's purely an us problem (in which case keeping the solution on our end is likely also appropriate), or whether others are also affected and would find this useful.

Thanks a lot for the whole writeup, it is interesting to understand the chain of events that brought you to this.

I am neutral (or slightly against) libc++ headers being includable when compiled as C. On the one hand, it does add flexibility, however it also encourages having poor build system hygiene. I think we're on the same page about this. In fact, I think ideally we would `#error` out when libc++ headers are used outside of C++ to fail fast and obviously, in the same spirit as D131441 <https://reviews.llvm.org/D131441>.

Another problem with libc++ headers being includable as C is that we'd have to actually test it and support it, and I suspect there's a bunch of stuff that doesn't work as intended in that configuration -- we're just not really aware of those problems.

That being said, above my preference, we do seem to have precedent for supporting this, see `libcxx/test/libcxx/include_as_c.sh.cpp`. It seems like I basically forgot to update that test when I added `<stdatomic.h>`.

I would hence suggest that we:

1. Update `include_as_c.sh.cpp`
2. Move the compiler detection block in `__config` to outside of `#if __cplusplus`
3. Not change `<stdatomic.h>`

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131435/new/

https://reviews.llvm.org/D131435