gh-858 10000 58: 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.
I would keep it 2-bit for a time.
There was a problem hiding this comment.
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.
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.
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.
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 ;-) |
|
Do I understand correctly that this changeset alters the alignment of I'm not at all familiar with the m68k issue that caused the addition of the padding field (nor what the implications might be on target platforms), but thought that I would ask for confirmation if the adjustment to the width of Edit: pondering it some more, I may have asked a very silly question here, since the difference of one bit isn't going to make a difference in terms of bytes, there will "just" be this odd bit not formally accounted for. |
|
@SnoopJeDi Nice catch. |
This padding was not updated when pythonGH-92579 reduced intern flag bits.
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).