help needed to verify MSVC preprocessor check in memoryview code

[Stefan Behnel]

Hi,

I stumbled over this code in MemoryView_C.c, line 36:

"""
#elif CYTHON_ATOMICS && MSC_VER
"""

Shouldn't this spell "defined(_MSV_VER)" ? Mind the leading underscore.

Could one of the MSVC users please verify with some memoryview code whether
this works or needs fixing? There is a macro "__PYX_DEBUG_ATOMICS" that you
can #define to make the C compiler issue a 'warning' for the code it uses
here, namely "Using MSVC atomics". Otherwise, you would see "Not using
atomics".

I considered simply changing it, but in the worst case, this might be code
that has never been used, so it might just break when enabling it. If both
versions work, I'd still like to change it because "_MSC_VER" is used
everywhere else and thus keeps the surprise level low.

Thanks!

Stefan

[Toni Barth]

Am 01.07.2015 um 08:52 schrieb Stefan Behnel:
> Hi,
>
> I stumbled over this code in MemoryView_C.c, line 36:
>
> """
> #elif CYTHON_ATOMICS && MSC_VER
> """
>
> Shouldn't this spell "defined(_MSV_VER)" ? Mind the leading underscore.
>

Hi,

I didn't check it with memoryview code, but _MSC_VER is definitely
correct. Checking the predefined constants page in msdn (german version
here (google doesn't even show me the english one)
https://msdn.microsoft.com/de-de/library/b0084kay.aspx), _MSC_VER is the
one and only way, a constant called MSC_VER doesn't even exist. I used
_MSC_VER in some of my code and I know that it works, so it would be
safer to use it instead of MSC_VER, even if that constant exists too, it
is nowhere listed and might therefore not be supported in all Visual
Studio versions.

Best Regards,

Toni

[Stefan Behnel]

'Toni Barth' via cython-users schrieb am 01.07.2015 um 09:35:
> Am 01.07.2015 um 08:52 schrieb Stefan Behnel:
>> I stumbled over this code in MemoryView_C.c, line 36:
>>
>> """
>> #elif CYTHON_ATOMICS && MSC_VER
>> """
>>
>> Shouldn't this spell "defined(_MSV_VER)" ? Mind the leading underscore.
>

> I didn't check it with memoryview code, but _MSC_VER is definitely correct.
> Checking the predefined constants page in msdn (german version here (google
> doesn't even show me the english one)
> https://msdn.microsoft.com/de-de/library/b0084kay.aspx), _MSC_VER is the
> one and only way, a constant called MSC_VER doesn't even exist. I used
> _MSC_VER in some of my code and I know that it works, so it would be safer
> to use it instead of MSC_VER, even if that constant exists too, it is
> nowhere listed and might therefore not be supported in all Visual Studio
> versions.

I committed the fix to latest master. Please give it some testing.

https://github.com/cython/cython/commit/fd6ed9d0773d30559af7abfeadd253d6c3e37f58

Stefan