PortAudio / portaudio

PortAudio is a cross-platform, open-source C language library for real-time audio input and output.
Other
1.47k stars 304 forks source link

Fix MSVC warning wd4146 unary minus operator applied to unsigned type #814

Closed RossBencina closed 1 year ago

RossBencina commented 1 year ago

Fixes:

wd4146: W2 unary minus operator applied to unsigned type, result still unsigned

See next comment for details.

RossBencina commented 1 year ago

CMake build / Windows MSVC (pull_request) action failed with:

 paqa.c
D:\a\portaudio\portaudio\qa\loopback\src\paqa.c(1354,108): error C2220: the following warning is treated as an error [D:\a\portaudio\portaudio\build\qa\loopback\paloopback.vcxproj]
D:\a\portaudio\portaudio\qa\loopback\src\paqa.c(1354,108): warning C4146: unary minus operator applied to unsigned type, result still unsigned [D:\a\portaudio\portaudio\build\qa\loopback\paloopback.vcxproj]

The offending expression is the -2147483648 here:

const int intInput[] = { 2147483647, 2147483647/2, -1073741824 /*-2147483648/2 doesn't work in msvc*/, -2147483648 };

First, note that sizeof(int) is 4 on 32-bit systems, and also on 64-bit Windows.

The problem is that you can't write the integer constant -2147483648 like that in C and expect to get a 32-bit int.

Referring the the ISO C90 grammar, -2147483648 parses as:

unary-operator('-', decimal_constant('2147483648'))

Then, based standard C rules, since 2147483648 does not fit in an int it is interpreted as being an unsigned int. So the warning is correct, the code tries to negate an unsigned int.

This situation is discussed at the following links:

Following the definition of INT_MIN on my machine, I propose that we write the number like this:

(-2147483647 - 1)

RossBencina commented 1 year ago

@dechamps just letting you know I think I've fixed this, incase you were tempted to look at it.

dechamps commented 1 year ago

Is there any reason why we can't change the code to use the standard constants such as INT_MIN, INT_MAX etc. instead of literal numbers? E.g. something like

const int intInput[] = { INT_MAX, INT_MAX/2, INT_MIN/2, INT_MIN };

This would seem like a cleaner/more readable way to avoid the warning.

RossBencina commented 1 year ago

@dechamps because we want INT32_MIN/INT32_MAX (i.e. always 32-bit values) irrespective of the size of int. And those symbols are not present in C89.