[WIP] bpo-35059: Add assertion to _PyObject_CAST() by vstinner · Pull Request #10649 · python/cpython

Conversation

@vstinner

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

https://bugs.python.org/issue35059

@vstinner

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.

serge-sans-paille

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.

@vstinner

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

@vstinner

cc @nascheme: you might like this experiment :-)

@vstinner

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

@serhiy-storchaka

I did not know about _PyObject_CAST().

@vstinner

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.

@vstinner

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.

Labels