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

gh-85858: Remove PyUnicode_InternImmortal() function #92579

Merged
merged 1 commit into from May 13, 2022

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented May 9, 2022

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).

@vstinner
Copy link
Member Author

@vstinner vstinner commented May 9, 2022

@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;
Copy link
Member

@serhiy-storchaka serhiy-storchaka May 10, 2022

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.

Copy link
Member Author

@vstinner vstinner May 11, 2022

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 **);
Copy link
Member

@serhiy-storchaka serhiy-storchaka May 10, 2022

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.

Copy link
Member

@methane methane May 10, 2022

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.

Copy link
Member Author

@vstinner vstinner May 11, 2022

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().
@vstinner
Copy link
Member Author

@vstinner vstinner commented May 12, 2022

I rebased my PR to fix merge conflicts.

@vstinner vstinner merged commit 059b5ba into python:main May 13, 2022
13 checks passed
@vstinner vstinner deleted the remove_immortal branch May 13, 2022
@vstinner
Copy link
Member Author

@vstinner vstinner commented May 13, 2022

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 ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants