Skip to content
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

Useless code block in math.trunc() ? #111417

Closed
skirpichev opened this issue Oct 28, 2023 · 3 comments
Closed

Useless code block in math.trunc() ? #111417

skirpichev opened this issue Oct 28, 2023 · 3 comments
Labels
extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error

Comments

@skirpichev
Copy link
Contributor

skirpichev commented Oct 28, 2023

Bug report

Bug description:

The math.trunc function has:

cpython/Modules/mathmodule.c

Lines 2073 to 2076 in f013b47

if (!_PyType_IsReady(Py_TYPE(x))) {
if (PyType_Ready(Py_TYPE(x)) < 0)
return NULL;
}

c.f. math.ceil or math.floor.

This seems to be a history artifact: the function was introduced in 400adb0 as a moved builtins.trunc(), while math.ceil/floor were here from the initial revision.

I think it's safe to remove mentioned block.

PS: Issue opened per @serhiy-storchaka suggestion in #110000.

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs

@Jason-Y-Z
Copy link
Contributor

Created a small PR to remove the unused code block - #111454

@serhiy-storchaka
Copy link
Member

I suggested @skirpichev to open a separate issue for discussion and reference. Previously this change was hidden in a pile of unrelated changes.

Now I think that it is safe. This check is immediately followed by _PyObject_LookupSpecial() which calls _PyType_Lookup() which, after looking in the type cache, calls find_name_in_mro() which looks up tp_mro, and, if it has not yet been set, make the type ready.

I think that a similar code in the builtin round() can be removed as well. Other uses of _PyType_IsReady() and PyType_Ready() look more justified.

@skirpichev
Copy link
Contributor Author

@serhiy-storchaka, in the end, it was your commit, that makes additional PyType_Ready() (since 15d3d04) call here useless: 8ef3460.

@Jason-Y-Z, I think you could reference that commit in you commit message. And remove similar code in round(), of course.

@iritkatriel iritkatriel added the extension-modules C modules in the Modules dir label Nov 26, 2023
Jason-Y-Z added a commit to Jason-Y-Z/cpython that referenced this issue Feb 3, 2024
serhiy-storchaka pushed a commit that referenced this issue Feb 3, 2024
…11454)

_PyObject_LookupSpecial() now ensures that the type is ready.
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
…pythonGH-111454)

_PyObject_LookupSpecial() now ensures that the type is ready.
fsc-eriker pushed a commit to fsc-eriker/cpython that referenced this issue Feb 14, 2024
…pythonGH-111454)

_PyObject_LookupSpecial() now ensures that the type is ready.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants