_Atomic use on C++ compilers can cause issues

[joerowell]

I've noticed that cysignals uses _Atomic where it is available (c.f 8073803).

In C land, this is fine.

However, for C++ this is more questionable, and I've recently run into such an issue.

Essentially, the problem is that when installing cysignals the compiler used is fixed. This means that anyone wanting to use a generated cysignals in their code must use a compatible compiler, or there will be mismatches.

For example, on my Mac, the default C++ compiler is clang, which defines _Atomic. So, when cysignals has its headers configured, it is configured to use _Atomic.

However, if I then try to compile some code that uses cysignals using g++, I get a series of hard-to-diagnose errors. These errors occur precisely because cysignals expects _Atomic to exist, but it doesn't in g++.

I think the most straightforward thing to do is to not use _Atomic for C++ code. I'm happy to provide such a fix: equally, if there's a good reason why _Atomic is to be preferred, I'm also happy to try and produce a fix that satisfies both sides.

[kliem]

Hi Joe,

using different compilers for C++ appears to be pretty unstable, see e.g. this post https://stackoverflow.com/questions/5728116/can-i-link-object-files-made-by-one-compile-to-those-made-by-another-one.

So my personal opinion is that one shouldn't expect this thing to work. However, it failing with hard-to-diagnose errors is certainly not nice. Instead one could just throw an error if the compiler used differs from the compiler used during configuration using the most popular compilers https://blog.kowalczyk.info/article/j/guide-to-predefined-macros-in-c-compilers-gcc-clang-msvc-etc..html.

The bigger problem I see here, is that we assume standard compilers for the wheels. So if someone always uses g++ on a mac, it still won't work from the installed wheels, as the installed wheels assumes clang I suppose.

A solution might be to allow configuring with multiple compilers. But I have no clue if it is safe to use different versions of cysignals. I guess it would be especially nice for the wheels built for Mac, as there both clang and gcc are popular.

In general, I'm guessing about a lot of this stuff.

Jonathan

[joerowell]

Hi! Thanks for the quick reply.

I agree that the problem is tricky. I'm surprised about the ABI requirements though: my understanding was that this shouldn't be a problem on the same operating system.

In any case, the problem I encountered wasn't caused by ABI problems: it was caused by trying to parse struct_signals.h. I guess this is because Cython needs the struct definition to be present on an interface boundary.

Is there a particular reason why cysignals uses _Atomic for both C and C++ code? The git blame seem to reference a particular Sage issue, but the reasoning seems similar to this issue :)

In any case, I'd be surprised if it caused any interface problems: there's only the single static struct in implementation.c is actually exposed to the outside world. It's also recommended that compilers make _Atomic(T) and std::atomic<T> layout compatible, at least according to the notes section of this.