[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[STINNER Victor]

STINNER Victor  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[STINNER Victor]

STINNER Victor  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[STINNER Victor]

STINNER Victor  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[STINNER Victor]

STINNER Victor  added the comment:

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.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[miss-islington]

Change by miss-islington :


--
nosy: +miss-islington
nosy_count: 5.0 -> 6.0
pull_requests: +22664
pull_request: https://github.com/python/cpython/pull/23804

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[STINNER Victor]

STINNER Victor  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[STINNER Victor]

STINNER Victor  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +22482
pull_request: https://github.com/python/cpython/pull/23614

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[Paul Ganssle]

Change by Paul Ganssle :


--
pull_requests: +19849
pull_request: https://github.com/python/cpython/pull/20624

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[Dong-hee Na]

Change by Dong-hee Na :


--
keywords: +patch
pull_requests: +19845
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/20619

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[Ammar Askar]

Ammar Askar  added the comment:

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

--
nosy: +ammar2

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

> 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 :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[Dong-hee Na]

Dong-hee Na  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

> 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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[Dong-hee Na]

Dong-hee Na  added the comment:

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

--
nosy: +corona10, pablogsal

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40686] Compiler warnings in _zoneinfo.c on Windows build in 64-bit

[STINNER Victor]

New submission from STINNER Victor :

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]

--
components: Build
messages: 369374
nosy: p-ganssle, vstinner
priority: normal
severity: normal
status: open
title: Compiler warnings in _zoneinfo.c on Windows build in 64-bit
versions: Python 3.10, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com