[WIP] bpo-35059: Add assertion to _PyObject_CAST() by vstinner · Pull Request #10649 · python/cpython
Conversation
_PyObject_CAST() and _PyVarObject_CAST() macros now require that the
type of the argument has a size. For example, _PyObject_CAST(frame)
now raises a compilation error if frame type is "struct _frame*" but
"struct _frame" is not defined.
_PyObject_CAST() and _PyVarObject_CAST() macros now raise a
compilation error if the argument is a "PyObject" or a "PyObject**".
Partially revert b37672d to show
that _PyObject_CAST() accepts non-trivial expressions.
This change introduces two backward incompatible changes:
_PyObject_CAST(keyfile ? keyfile_bytes : certfile_bytes)raises a compilation error, whereas it's legit code and works if Py_TYPE() is a regular function._PyObject_CAST(frame)raises a compilation error if frame type is undefined. Example:struct _frame *without "frameobject.h".
See my commit b37672d for other examples of regressions.
So I don't think that this change should be merged.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if op is an expression, then your dereferencing is likely to fail, better use *(op).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And you don't have the issue of op being evaluated three times because sizeof does not evaluate the expression \o/
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, fixed.
_PyObject_CAST() and _PyVarObject_CAST() macros now require that the type of the argument has a size. For example, _PyObject_CAST(frame) now raises a compilation error if frame type is "struct _frame*" but "struct _frame" is not defined. _PyObject_CAST() and _PyVarObject_CAST() macros now raise a compilation error if the argument is a "PyObject" or a "PyObject**". Partially revert b37672d to show that _PyObject_CAST() accepts non-trivial expressions.
_PyObject_CAST() and _PyVarObject_CAST() macros now require that the
type of the argument has a size. For example, _PyObject_CAST(frame)
now raises a compilation error if frame type is "struct _frame*" but
"struct _frame" is not defined.
IMHO it's a blocker issue, it's a backward incompatible change.
At least, _PyObject_CAST() doesn't require 'op' to have a size of Py_LIMITED_API is defined.
cc @nascheme: you might like this experiment :-)
@serhiy-storchaka, @pablogsal: I experimented a C macro to detect "Py_TYPE(&obj)" bug (with "PyObject *obj"), but it doesn't seem possible to write a magic macro because the Python C API is a mess :-)
For example, _PyObject_CAST() should also accept PyTupleObject* which contains a "PyObject ob_base". But there are other type monsters like:
- PyASCIIObject: PyObject ob_base;
- PyCompactUnicodeObject: PyASCIIObject _base;
- PyUnicodeObject: PyCompactUnicodeObject _base;
I did not know about _PyObject_CAST()
I added it yesterday :-) commit 2ff8fb7
I wrote it for C macros to avoid "(PyObject*)ob" mistake: missing parenthesis around "ob". By the way, I should fix Include/odictobject.h in Python 3.6 and 3.7.
I have to confess that I also wrote the macro to be able to make such experiment. More generally, it's a hint on cast between object pointer types which helps me on my work on the C API.
Ok, this PR was fun to write, but since it's breaking the backward compatibility, it's really a no-go and so I close it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters