-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
bpo-46311: Clean up PyLong_FromLong and PyLong_FromLongLong #30496
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
Conversation
I don't see how this qualifies for skip issue. Perhaps I am misreading the devguide, but to me this PR seems inappropriately labelled. If I am mistaken; sorry for the noise. |
@erlend-aasland Yes, I think you're right. I'll add an issue. |
@@ -274,44 +274,40 @@ _PyLong_Negate(PyLongObject **x_p) | |||
} | |||
|
|||
/* Create a new int object from a C long int */ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a minor consistency fix, following the Boy Scout Rule: the prevailing style in this file is to leave a blank line between the comment-description and the definition. But the change is not strictly necessary for this PR, and I can revert if reviewers prefer.
Thanks. Looks good on initial review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two changes that should silence compiler warnings.
Thanks, @markshannon. The compiler warnings have been silenced. |
Thanks |
PR #27832 inadvertently introduced a couple of changes to
PyLong_FromLong
that didn't make a lot of sense: an(unsigned long)
cast was replaced with(twodigits)
, and a digit count variable (counting number of PyLong digits in a C long) had its type needlessly changed fromint
toPy_ssize_t
.(unsigned long)
cast is obviously correct, while figuring out whether(twodigits)
loses information takes some work.long
.This PR:
ival < 0 ? 0U-(unsigned long)ival : ival
) gets compiled to something branchless on most platforms, as doesival < 0 ? -ndigits : ndigits
PyLong_FromLongLong
, which now has a fast path for medium-size values.https://bugs.python.org/issue46311