8000 GCC -Wsign-conversion compiler warning for Include/cpython/longintrepr.h · Issue #112353 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

GCC -Wsign-conversion compiler warning for Include/cpython/longintrepr.h #112353

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
stratakis opened this issue Nov 23, 2023 · 2 comments
Closed
Labels
type-bug An unexpected behavior, bug, or error

Comments

@stratakis
Copy link
Contributor
stratakis commented Nov 23, 2023

Bug report

Bug description:

When testing cffi with Python 3.12 on Centos Stream 8, with GCC version 8.5.0, the tests were failing as they are using -Werror and -Wsign-conversion.

More specifically they were failing with:

./Include/cpython/longintrepr.h: In function ‘_PyLong_CompactValue’:
./Include/cpython/longintrepr.h:121:23: warning: conversion to ‘Py_ssize_t’ {aka ‘long int’} from ‘long unsigned int’ may change the sign of the result [-Wsign-conversion]
     Py_ssize_t sign = 1 - (op->long_value.lv_tag & _PyLong_SIGN_MASK);

The offending line was introduced in 9392379 for Python> = 3.12

However that wasn't the case with later compiler versions so after a little digging I was pointed to https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=c77074d05691053ee7347d9e44ab89b3adb23fb1 and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=40752

That means that for GCC >= 10, the same warning can be seen again by using -Wsign-conversion and -Warith-conversion together.

The issue is easily fixed by casting to Py_ssize_t however, as indicated by the gcc bugzilla, it's debatable if there should even be a warning for those cases.

Would a fix even be considered for this case?

See also: #112301

CPython versions tested on:

3.12, 3.13, CPython main branch

Operating systems tested on:

Linux

@stratakis stratakis added the type-bug An unexpected behavior, bug, or error label Nov 23, 2023
@stratakis
Copy link
Contributor Author

cc @encukou

@encukou
Copy link
Member
70FE encukou commented Nov 27, 2023

I don't see any value in working around a warning that was removed in later versions of the compiler. Especially when there are only two cases to consider, and both are tested.
If this starts warning again in the future, let's add a cast at that point.

Tightening the default warnings could be considered, bit that'd be a different issue.

@encukou encukou closed this as completed Nov 27, 2023
@encukou encukou closed this as not planned Won't fix, can't repro, duplicate, stale Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants
0