c:\vstinner\python\3.9\modules\_zoneinfo.c(903): warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss of data [C:\vstinner\python\3.9\PCbuild\_ 
zoneinfo.vcxproj]
c:\vstinner\python\3.9\modules\_zoneinfo.c(904): warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss of data [C:\vstinner\python\3.9\PCbuild\_ 
zoneinfo.vcxproj]
c:\vstinner\python\3.9\modules\_zoneinfo.c(1224): warning C4068: unknown pragma [C:\vstinner\python\3.9\PCbuild\_zoneinfo.vcxproj]
c:\vstinner\python\3.9\modules\_zoneinfo.c(1225): warning C4068: unknown pragma [C:\vstinner\python\3.9\PCbuild\_zoneinfo.vcxproj]
c:\vstinner\python\3.9\modules\_zoneinfo.c(1227): warning C4068: unknown pragma [C:\vstinner\python\3.9\PCbuild\_zoneinfo.vcxproj]
c:\vstinner\python\3.9\modules\_zoneinfo.c(1770): warning C4244: '=': conversion from 'ssize_t' to 'uint8_t', possible loss of data [C:\vstinner\python\3.9\PCbuild\_zon 
einfo.vcxproj]
c:\vstinner\python\3.9\modules\_zoneinfo.c(2408): warning C4028: formal parameter 2 different from declaration [C:\vstinner\python\3.9\PCbuild\_zoneinfo.vcxproj]

@pablogsal
GH-20342 looks like related to this issue.
Can you update the current status?

> Can you update the current status?

Sorry, what do you mean by "current status"? Do you meant this issue or the status of that PR?

Two things.

1. GH-20342 solved this issue?
2. If not what's left for this issue? :)

Sorry, Normally I should check the current status.
But I don't have Windows machine so I can not test it.
However the update will help contributors who want to deal with :)

Thanks Pablo

> 1. GH-20342 solved this issue?

As far as I understand, yes. Although I did not check if all the warnings here are solved in that PR, as the PR eliminates all the ones that we found, it should be resolved.

I think we can close this issue

> Thanks Pablo

Thanks to you for all the help :)

There's still the "unknown pragma" warnings left, I pinged p-ganssle about it in the zoneinfo commit but it should probably be guarded like the ones in the ssl module:

https://github.com/python/cpython/blob/a871f692b4a2e6c7d45579693e787edc0af1a02c/Modules/_ssl.c#L46

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.

New changeset aefb69b23f056c61e82ad228d950f348de090c70 by Victor Stinner in branch 'master':
bpo-40686: Fix compiler warnings on _zoneinfo.c (GH-23614)
https://github.com/python/cpython/commit/aefb69b23f056c61e82ad228d950f348de090c70

The 3 compiler warnings are now fixed in master, and a 3.9 backport will land as soon as the CI tests pass. Thanks everyone who helped to fix these warnings.

I know that the solution is not perfect, but we have to deal with C language limitations.

Fixing these compiler warnings helps PR 18532 "[workflow] Use MSVC problem matcher for Windows action build".

New changeset 7492b55ea0ca993c353b6373341b92e40faa9c4d by Miss Islington (bot) in branch '3.9':
bpo-40686: Fix compiler warnings on _zoneinfo.c (GH-23614) (GH-23804)
https://github.com/python/cpython/commit/7492b55ea0ca993c353b6373341b92e40faa9c4d

Paul created a follow-up issue: bpo-42660.