bpo-45476: Disallow using PyFloat_AS_DOUBLE() as l-value

[vstinner]

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.

https://bugs.python.org/issue45476

[gareth-rees]

Typo: "expressing" for "expression"

[vstinner]

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

[vstinner]

I rebased the PR and reverted PyBytes_AS_STRING() and PyByteArray_AS_STRING() changes.

[vstinner]

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

[encukou]

Is there any deprecation warning for these changes?

[vstinner]

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:

$ ./search_pypi_top_5000.sh '(PyByteArray|PyBytes|PyCFunction|PyDict|PyFunction|PyHeapType|PyInstanceMethod|PyList|PyMemoryView|PyMethod|PySet|PyTuple|PyWeakref)_GET[a-zA-Z0_9_]+ *([a-zA-Z0-9_]+) *=[^<>=]'
pypi-top-5000_2021-08-17/Cython-0.29.24.tar.gz

$ ./search_pypi_top_5000.sh 'PyFloat_AS_DOUBLE.*[^<>=!]=[^<>=]'
# no result

Cython-0.29.24.tar.gz is a false positive because of my weak regex:

Cython-0.29.2: Cython/Compiler/Optimize.py:    PyBytes_GET_SIZE_func_type = PyrexTypes.CFuncType(

[vstinner]

Is there any deprecation warning for these changes?

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

[vstinner]

@rhettinger: You may want to review the updated PR. I made minor changes since you approved the PR.

[encukou]

The SC decision you linked says:

these changes must be announced better and in general the deprecation notice needs to stay for at least two releases in the documentation.

How does this PR do that?

[vstinner]

Documentation of the modified macros:

• https://docs.python.org/dev/c-api/bytearray.html#c.PyByteArray_GET_SIZE "Macro version of PyByteArray_Size()."
• https://docs.python.org/dev/c-api/bytes.html#c.PyBytes_GET_SIZE "Macro form of PyBytes_Size() but without error checking."
• https://docs.python.org/dev/c-api/float.html#c.PyFloat_AS_DOUBLE
• https://docs.python.org/dev/c-api/method.html#c.PyInstanceMethod_GET_FUNCTION "Macro version of PyInstanceMethod_Function() which avoids error checking."
• https://docs.python.org/dev/c-api/list.html#c.PyList_GET_SIZE "Macro form of PyList_Size() without error checking."
• https://docs.python.org/dev/c-api/memoryview.html#c.PyMemoryView_GET_BASE
• https://docs.python.org/dev/c-api/memoryview.html#c.PyMemoryView_GET_BUFFER
• https://docs.python.org/dev/c-api/method.html#c.PyMethod_GET_FUNCTION "Macro version of PyMethod_Function() which avoids error checking."
• https://docs.python.org/dev/c-api/method.html#c.PyMethod_GET_SELF "Macro version of PyMethod_Self() which avoids error checking."
• https://docs.python.org/dev/c-api/set.html#c.PySet_GET_SIZE "Macro form of PySet_Size() without error checking."
• https://docs.python.org/dev/c-api/tuple.html#c.PyTuple_GET_SIZE
• https://docs.python.org/dev/c-api/weakref.html#c.PyWeakref_GET_OBJECT "Similar to PyWeakref_GetObject(), but implemented as a macro that does no error checking."

The following functions are not documented:

• PyCFunction_GET_CLASS()
• PyCFunction_GET_FLAGS()
• PyCFunction_GET_FUNCTION()
• PyCFunction_GET_SELF()
• PyDict_GET_SIZE()
• 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()

[vstinner]

Is there any deprecation warning for these changes?

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 PyFloat_AS_DOUBLE() function, it's less clear to me what is the intended usage. A Python float object is immutable. Once it's created, it can be referenced at multiple places (stored in two lists). If a list stores the number 1.0, it would be surprising to modify it in-place somewhere else to change its value to 2.0. Maybe some weird projects use an in-place modification when the reference count is 1, to avoid the cost of creating a new object. It sounds dangerous but people are creative to abuse the C API :-) What reassures me is that in the PyPI top 5000, I cannot find a single project doing that.

[encukou]

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.
Changing old API is tricky because you don't usually know how it was documented/used in the past -- it's entirely possible that reasonable people looked at the code rather than the documentation.

[vstinner]

I wrote PEP 674 "Disallow using macros as l-value" for this change: https://python.github.io/peps/pep-0674/