bpo-9566 & bpo-30747: Silence warnings from pyatomic.h macros

[segevfiner]

Apparently MSVC is too stupid to understand that the alternate branch is not taken and emits a warning for it:

warning C4133: 'function': incompatible types - from 'volatile uintptr_t *' to 'volatile int *'

Warnings added in #2383.

cc @Paxxi @pitrou

https://bugs.python.org/issue9566

[pitrou]

Instead of adding obscure pragmas, wouldn't be it more readable to add the necessary casts?
(though of course the end result -- suppressing the warnings -- would be the same)

[segevfiner]

Instead of adding obscure pragmas, wouldn't be it more readable to add the necessary casts?
(though of course the end result -- suppressing the warnings -- would be the same)

How can I cast to ignore the warning? The problem, to my understanding, is that MSVC thinks that the other branch of the if or ternary can be taken for some reason. So, for example, if we pass a volatile uintptr_t*, it thinks _Py_atomic_load_32bit can be called, so it emits the warning. But maybe I'm wrong...

[pitrou]

Simply cast the relevant parameters to _Py_atomic_load_32bit? Perhaps I'm missing something.

[segevfiner]

@pitrou Done. Used volatile long long* and volatile long* since that's what the Interlocked* functions take.

[Paxxi]

I was just writing about that solution when I saw the commits being pushed :)

I think this is much saner, possibly use uintptr_t instead of long long just to keep it consistent?

Thank you for looking into this!

[segevfiner]

uintptr_t is 32-bit in a 32-bit build which you will than pass to _Py_atomic_load_64bit...

[Paxxi]

there's another macro for 32bit which doesn't contain the ternary operator as there's no 64bit operations required there.

[segevfiner]

there's another macro for 32bit which doesn't contain the ternary operator as there's no 64bit operations required there.

I think the _Py_atomic_store_explicit macro I modified is used for both 32-bit and 64-bit MSVC builds (Include/pyatomic.h:234). _Py_atomic_store_64bit is defined to just return the value directly on 32-bit builds (Include/pyatomic.h:266).

There is another similar _Py_atomic_store_explicit below but I think that one is for Windows on ARM...

[Paxxi]

seems I remembered it wrong. You're entirely correct.