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 — bpo-45476: Disallow using PyFloat_AS_DOUBLE() as l-value by vstinner · Pull Request #28976 · python/cpython Conversation
|
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Typo: "expressing" for "expression"
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I wrote PEP 670 https://www.python.org/dev/peps/pep-0670/ to decide what's the best approach to fix https://bugs.python.org/issue45476 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I rebased the PR and reverted PyBytes_AS_STRING() and PyByteArray_AS_STRING() changes. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
The following "GET" and "AS" functions can no longer be used as l-value (to modify a Python object): * PyByteArray_GET_SIZE() * PyBytes_GET_SIZE() * PyCFunction_GET_CLASS() * PyCFunction_GET_FLAGS() * PyCFunction_GET_FUNCTION() * PyCFunction_GET_SELF() * PyDict_GET_SIZE() * PyFloat_AS_DOUBLE() * PyFunction_GET_ANNOTATIONS() * PyFunction_GET_CLOSURE() * PyFunction_GET_CODE() * PyFunction_GET_DEFAULTS() * PyFunction_GET_GLOBALS() * PyFunction_GET_KW_DEFAULTS() * PyFunction_GET_MODULE() * PyHeapType_GET_MEMBERS() * PyInstanceMethod_GET_FUNCTION() * PyList_GET_SIZE() * PyMemoryView_GET_BASE() * PyMemoryView_GET_BUFFER() * PyMethod_GET_FUNCTION() * PyMethod_GET_SELF() * PySet_GET_SIZE() * PyTuple_GET_SIZE() * PyWeakref_GET_OBJECT() These macros are modified to use the _Py_RVALUE() macro.
|
@corona10 @erlend-aasland @encukou: Would you mind to review this change? It's a C API incompatible change made on purpose to aid detecting bugs in C extensions when the C API is misused. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Is there any deprecation warning for these changes? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I cannot find any of the 25 macros being used as a l-value in an assignement in the PyPI top 5000 packages. So the risk of breaking 3rd party packages is very low. I searched for the following patterns: Cython-0.29.24.tar.gz is a false positive because of my weak regex: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
No. Which kind of warning do you expect? I am not aware of any technical way to emit a compiler warning when a macro is only used as an l-value, see previous discussion about the Py_TYPE() change: python/steering-council#79 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@rhettinger: You may want to review the updated PR. I made minor changes since you approved the PR. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
The SC decision you linked says:
How does this PR do that? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Documentation of the modified macros:
The following functions are not documented:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
For me, using these macros as l-value is an unintentional usage. I don't think that we have to go through a deprecation process for this specific corner case. For "GET" functions, it sounds weird to me to use them to set an object attribute or to modify a Python object. For the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I like to think that the target audience for deprecation messages and "versionchanged" notices is someone who inherited a decades-old legacy project, and is tasked with keeping it usable without knowing much about Python and current best practices. If it works for that audience, it should work for everyone else as well. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I wrote PEP 674 "Disallow using macros as l-value" for this change: https://python.github.io/peps/pep-0674/ |
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.