-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
bpo-29878: Add global instances of int for 0 and 1. #852
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
bpo-29878: Add global instances of int for 0 and 1. #852
Conversation
@serhiy-storchaka, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tiran, @ncoghlan and @benjaminp to be potential reviewers. |
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.
LGTM
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.
I'm really happy to see all these code go away! It's painful to have to check for errors on obvious things like that numbers 0 and 1!
The overall change LGTM, but IMHO it would be safer to use Py_CLEAR() in PyLong_Fini(). I have propose a minor coding style change.
Objects/longobject.c
Outdated
return -1; | ||
} | ||
*pdiv = (PyLongObject*)_PyLong_Zero; | ||
Py_INCREF(_PyLong_Zero); |
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.
Whenever possible, even if we are protected by the GIL, I prefer to have Py_INCREF before assignement, for consistency.
Objects/longobject.c
Outdated
@@ -5520,6 +5513,8 @@ PyLong_Fini(void) | |||
/* Integers are currently statically allocated. Py_DECREF is not | |||
needed, but Python must forget about the reference or multiple | |||
reinitializations will fail. */ | |||
Py_DECREF(_PyLong_One); | |||
Py_DECREF(_PyLong_Zero); |
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.
Py_CLEAR() would be safer if PyLong_Init() is called again (Py_Initialized called multiple times), no?
Objects/rangeobject.c
Outdated
return NULL; | ||
} | ||
start = _PyLong_Zero; | ||
Py_INCREF(start); |
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.
ditto, in general I prefer to start with INCREF.
For internal use only.