This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author vstinner
Recipients ammar2, corona10, p-ganssle, pablogsal, vstinner
Date 2020-12-16.15:25:07
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1608132307.93.0.805711682637.issue40686@roundup.psfhosted.org>
In-reply-to
Content
The problem can be simplified to this x.c file:
---
static int invalid_day(unsigned int day)
{
    return (day < 0 || day > 6);
}

int main()
{
    invalid_day(3);
    return 0;
}
---

GCC emits the warning:

$ gcc x.c -o x -O3 -Wall -Wextra
x.c: In function 'invalid_day':
x.c:3:17: warning: comparison of unsigned expression in '< 0' is always false [-Wtype-limits]
    3 |     return (day < 0 || day > 6);
      |                 ^

There are different options to avoid the warning:


(A) Remove "day < 0" test

Easiest option, portable, simple: my PR 23614.


(B) Disable compiler warnings on the test

Solution currently implemented with pragma + PR 20619 to fix pragmas.


(C) Cast the 'day' variable to a signed type

I understand that Paul wants the code to be as generic as possible, and not depending on the "day" parameter type. For example, casting to "int8_t" may introduce a risk of integer overflow if day type is larger than 8 bits. Not my favorite option.


(D) Make "day < 0" conditional depending if day type is signed or not
(E) Check that day type is unsigned to ensure indirectly that "day >= 0"

Checking if *a type* is signed or not is easy using the C preprocessor:

#define _Py_IS_TYPE_UNSIGNED(type) (((type)-1) > (type)0) 

The problem is that there is no standard function to get a variable type. GCC and clang provide the __typeof__(var) extension, C++ provides decltype(var) (but CPython code base cannot be built with a C++ compiler if I recall correctly).

Paul's PR 20624 introduces Py_ASSERT_VAR_UNSIGNED(var) macro which fails during compilation if the variable is unsigned, or does nothing if the compiler doesn't provide a way to get a variable type (ex: MSC on Windows).


--


Most answers about "comparison of unsigned expression always false" question on the Internet are (A): remove the check which emits the warning.

My worry is also that outside _zoneinfo.c, they are tons of functions which rely on the fact that an unsigned type cannot be negativ. I don't want to start adding Py_ASSERT_VAR_UNSIGNED(). For me, it's part of the C language and there is no need to be explicit about it. If a developer changes a variable type, they have to check the type bounds and check of the variable is used.

I would prefer to be consistent and never check for "< 0" if the type is unsigned, nor ensure with an assertion that the type is unsigned.

Paul is in disagreement with that.
History
Date User Action Args
2020-12-16 15:25:07vstinnersetrecipients: + vstinner, ammar2, p-ganssle, corona10, pablogsal
2020-12-16 15:25:07vstinnersetmessageid: <1608132307.93.0.805711682637.issue40686@roundup.psfhosted.org>
2020-12-16 15:25:07vstinnerlinkissue40686 messages
2020-12-16 15:25:07vstinnercreate