GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||
GeezerVison is showing github.com — [3.14] gh-146056: Fix repr() for lists and tuples containing NULLs (GH-146129) by serhiy-storchaka · Pull Request #146155 · python/cpython Conversation
|
|
|
||
| Call :c:func:`PyObject_Repr` on *obj* and write the output into *writer*. | ||
|
|
||
| If *obj* is ``NULL``, write the string ``"<NULL>"`` into *writer*. |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to not change PyUnicodeWriter_WriteRepr() in a Python 3.14.x bugfix release. Can you leave it unchanged? Instead, modify list_repr() and tuple_repr() to write <NULL> string explicitly.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
We need to modify not only list_repr() and tuple_repr(), but all code that uses PyUnicodeWriter_WriteRepr(). And what about the user code? We leave it with a mine which explodes in rare and most likely not tested case.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
In the Python code base, if I read the code correctly, only tuple_repr() can call PyUnicodeWriter_WriteRepr() with NULL. For example, list_repr() cannot pass NULL, because item = Py_NewRef(v->ob_item[i]) crash if the item is NULL.
And what about the user code? We leave it with a mine which explodes in rare and most likely not tested case.
My concern is that if you develop on (the future) Python 3.14.4 with PyUnicodeWriter_WriteRepr() which accepts NULL, you can get crashes if an user uses Python 3.14.0 which doesn't accept NULL. I don't think that it's a good idea to change the API in a 3.14.x bugfix release.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Python 3.14.0 is broken. It crashes when you call repr() for uninitialized list or tuple, and it can crash in other cases. Does it mean that we should not backport the bugfix? I think that it is the nature of bugfix releases that they make the code which failed in earlier releases to pass.
The documentation says that PyUnicodeWriter_WriteRepr() calls PyObject_Repr() on obj and writes the output into writer. PyObject_Repr() works with NULL.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
If you insist to change PyUnicodeWriter_WriteRepr() API, please add a versionchanged markup to document that the API changed.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
If we don't fix it in 3.14, we will need a versionchanged for 3.15. But we rarely add versionchanged for bugfixes, especially if the feature was added in the same version.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I'm suggesting to add a versionchanged markup in the 3.14 change to say that the API changed in Python 3.14.x.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Done. Created also a PR for adding in in main.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Ok, let's do that, accept NULL in PyUnicodeWriter_WriteRepr(). At least, the API change is now documented.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Note that no |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Sorry, @serhiy-storchaka, I could not cleanly backport this to |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…LLs (pythonGH-146129) (pythonGH-146155) (cherry picked from commit 0f2246b) (cherry picked from commit 7965133) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Victor Stinner <[email protected]>
…GH-146129) (pythonGH-146155) (cherry picked from commit 0f2246b) (cherry picked from commit 7965133) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Victor Stinner <[email protected]>
|
GH-146271 is a backport of this pull request to the 3.13 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.