clang-cl issues many warnings when building on Windows #131296
Comments
work around unimportant clang-cl warnings in blake2module.c
|
Let's not backport those because I suspect it will sometimes be hard (hence the "feature") |
fix warning : integer literal is too large to be represented in a signed integer type, interpreting as unsigned [-Wimplicitly-unsigned-literal]
|
Oh sure, no backport - I'd consider this a (minor) feature, anyway. Are warnings generally treated as bug issues here? |
|
thanks! all three of those should merge. actually looking at compiler warnings and minimizing them is a good thing... we do have some CI tests that check for changes in number of warnings on other platforms, if we get clang-cl in CI, even non-blocking, we should add the same for it. ex the "Check compiler warnings" ones in I do consider uninvestigated warnings to be bugs. But when it isn't a https://peps.python.org/pep-0011/ tier platform arguing for it can be hard. I think clang-cl is close enough even if not on a tier yet as we obviously already care about clang warnings on other platforms that its value is demonstrable. |
|
Yupp. I fully agree. My main motivation is, that clang-cl "sees all the Windows code" for which MSVC with the currently selected warning level produces no warnings. Some are really easy to fix, and will no longer cover the "real" ones like #131020. |
|
Ups, did you close the issue on purpose? Still a lot of warnings to go ... I am not that fast in creating the PRs :) |
|
I've created three more PRs. Still more than 40 warnings to go ... |
…ython#131299) work around unimportant clang-cl warnings in blake2module.c
…H-131297) fix warning : integer literal is too large to be represented in a signed integer type, interpreting as unsigned [-Wimplicitly-unsigned-literal]
…ythonGH-131300) fix unused warnings in pyshellext.cpp
Change MAX_NFRAME type from 'unsigned long' to size_t. On Windows 64-bit, 'unsigned long' is only 32-bit, whereas size_t is 64-bit.
Always set MAX_NFRAME to UINT16_MAX. Avoid the complicated code which emitted a compiler warning.
|
Sorry, I wasn't aware of this issue when I wrote #131487. I just noticed some warnings on a CI on one of my PR and I wanted to reduce the number of warnings to help me to distinguish if the warnings were introduced by my change or not. |
Also removes warnings suppression for MSVC that no longer need to be suppressed.
…ython#131299) work around unimportant clang-cl warnings in blake2module.c
…H-131297) fix warning : integer literal is too large to be represented in a signed integer type, interpreting as unsigned [-Wimplicitly-unsigned-literal]
…ythonGH-131300) fix unused warnings in pyshellext.cpp
Always set MAX_NFRAME to UINT16_MAX. Avoid the complicated code which emitted a compiler warning.
…onGH-131595) fix clangcl warning
…-131897) Also removes warnings suppression for MSVC that no longer need to be suppressed.
…piling with clang-cl (pythonGH-131593)
…to actual occurrence (pythonGH-131900)
See e.g. https://github.com/python/cpython/actions/runs/13681800146/job/38255779088.
I went through roughly 100 warnings to find this one #131020.
Others are mostly benign, yet I'd like to reduce them.
Since many of the warnings stem from
pythoncore, we see them twice, because the_freeze_modulecompiles a lot of those*.cfiles, too - and in case of PGO we see them a third time in the PGUpdate phase.E.g. a bunch of warnings will disappear after the next sync with hacl-star upstream, since @msprotz thankfully already merged hacl-star/hacl-star#1028.
I think it is best to do them in small PRs per
*.cfile.Ideally, I'd like to end up with the same warning configuration as used on Linux. When I configure for clang in WSL, I get
This would currently generate even more warnings, since we currently use
cpython/PCbuild/pyproject-clangcl.props
Line 41 in faa80fc
So maybe I should do this after a first round of cleaning?
Linked PRs
posixmodule.c#133142The text was updated successfully, but these errors were encountered: