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
gh-85858: Remove PyUnicode_InternImmortal() function #92579
Conversation
@methane @serhiy-storchaka: Would you mind to review this change? I reused #85858 issue. |
If interned != SSTATE_NOT_INTERNED, the two references from the | ||
dictionary to this object are *not* counted in ob_refcnt. | ||
*/ | ||
unsigned int interned:2; |
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 would keep it 2-bit for a time.
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.
PyASCIIObject is not part of the stable ABI. Why do you care about keeping 2 bits for interned?
@@ -265,10 +265,6 @@ PyAPI_FUNC(PyObject *) PyUnicode_InternFromString( | |||
const char *u /* UTF-8 encoded string */ | |||
); | |||
|
|||
// PyUnicode_InternImmortal() is deprecated since Python 3.10 | |||
// and will be removed in Python 3.12. Use PyUnicode_InternInPlace() instead. | |||
Py_DEPRECATED(3.10) PyAPI_FUNC(void) PyUnicode_InternImmortal(PyObject **); |
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.
Is not 2 versions too short period for removing from the public C API? I think we should be more conservative in such cases and prolong the deprecation to at least 4 years.
I would keep it as an alias of PyUnicode_InternInPlace
which may print a deprecation warning.
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.
There are no use from top4000 packages.
https://github.com/hpyproject/top4000-pypi-packages/search?q=PyUnicode_InternImmortal
We can remove it in early alpha and resurrect it if some packages are broken by this.
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.
Is not 2 versions too short period for removing from the public C API?
I followed PEP 387 process.
I think we should be more conservative in such cases and prolong the deprecation to at least 4 years.
Why do you care of this function in specific? Are you aware of projects using it?
There are no use from top4000 packages.
Same when I looked in December 2021:
#85858 (comment)
History of this function: #85858 (comment)
We can remove it in early alpha and resurrect it if some packages are broken by this.
Sure. I did that multiple times and I'm fine with reverting if it causes too many issues.
Removing the function early in 3.12 dev cycle gives more time to users to detect if their project is affected, to give them more time to decide how to update their code, or to ask to revert the change.
Remove the PyUnicode_InternImmortal() function and the SSTATE_INTERNED_IMMORTAL macro. The PyUnicode_InternImmortal() function is still exported in the stable ABI. The function is removed from the API. PyASCIIObject.state.interned size is now a single bit, rather than 2 bits. Keep SSTATE_NOT_INTERNED and SSTATE_INTERNED_MORTAL macros for backward compatibility, but no longer use them internally since the interned member is now a single bit and so can only have two values (interned or not interned). Update stats of _PyUnicode_ClearInterned().
I rebased my PR to fix merge conflicts. |
I merged my PR. Thanks for your review @methane. @serhiy-storchaka: I understand your concerns. I tried to reply to them. I rely on @methane's approval to merge my change ;-) |
Remove the PyUnicode_InternImmortal() function and the
SSTATE_INTERNED_IMMORTAL macro.
The PyUnicode_InternImmortal() function is still exported in the
stable ABI. The function is removed from the API.
PyASCIIObject.state.interned size is now a single bit, rather than 2
bits.
Keep SSTATE_NOT_INTERNED and SSTATE_INTERNED_MORTAL macros for
backward compatibility, but no longer use them internally since the
interned member is now a single bit and so can only have two values
(interned or not interned).