Skip to content

bpo-33738: Fix macros which contradict PEP 384#7585

Merged
ned-deily merged 2 commits intopython:masterfrom
pyside:master-macrofix
Jun 10, 2018
Merged

bpo-33738: Fix macros which contradict PEP 384#7585
ned-deily merged 2 commits intopython:masterfrom
pyside:master-macrofix

Conversation

@ctismer
Copy link
Copy Markdown
Contributor

@ctismer ctismer commented Jun 10, 2018

During development of the limited API support for PySide,
we saw an error in a macro that accessed a type field.

This patch fixes the 7 errors in the Python headers.
Macros which were not written as capitals were implemented
as function.

To do the necessary analysis again, a script was included that
parses all headers and looks for "->tp_" in sections which can
be reached with active limited API.

It is easily possible to call this script as a test.

Error listing:

../../Include/objimpl.h:243
(Py_TYPE(o)->tp_is_gc == NULL || Py_TYPE(o)->tp_is_gc(o)))
Action: commented only

../../Include/objimpl.h:362
Action: commented only

../../Include/objimpl.h:364
((PyObject **) (((char *) (o)) + Py_TYPE(o)->tp_weaklistoffset))
Action: commented only

../../Include/pyerrors.h:143
((char )(((PyTypeObject)(x))->tp_name))
Action: implemented function

../../Include/abstract.h:593
((obj)->ob_type->tp_iternext != NULL &&
(obj)->ob_type->tp_iternext != &_PyObject_NextNotImplemented)
Action: implemented function

../../Include/abstract.h:713
((obj)->ob_type->tp_as_number != NULL &&
(obj)->ob_type->tp_as_number->nb_index != NULL)
Action: implemented function

../../Include/abstract.h:924
( Py_TYPE(o)->tp_as_sequence->sq_item(o, i) )
Action: commented only

https://bugs.python.org/issue33738

During development of the limited API support for PySide,
we saw an error in a macro that accessed a type field.

This patch fixes the 7 errors in the Python headers.
Macros which were not written as capitals were implemented
as function.

To do the necessary analysis again, a script was included that
parses all headers and looks for "->tp_" in sections which can
be reached with active limited API.

It is easily possible to call this script as a test.

Error listing:

../../Include/objimpl.h:243
    (Py_TYPE(o)->tp_is_gc == NULL || Py_TYPE(o)->tp_is_gc(o)))
Action: commented only

../../Include/objimpl.h:362
Action: commented only

../../Include/objimpl.h:364
    ((PyObject **) (((char *) (o)) + Py_TYPE(o)->tp_weaklistoffset))
Action: commented only

../../Include/pyerrors.h:143
     ((char *)(((PyTypeObject*)(x))->tp_name))
Action: implemented function

../../Include/abstract.h:593
    ((obj)->ob_type->tp_iternext != NULL && \
     (obj)->ob_type->tp_iternext != &_PyObject_NextNotImplemented)
Action: implemented function

../../Include/abstract.h:713
    ((obj)->ob_type->tp_as_number != NULL &&            \
     (obj)->ob_type->tp_as_number->nb_index != NULL)
Action: implemented function

../../Include/abstract.h:924
    ( Py_TYPE(o)->tp_as_sequence->sq_item(o, i) )
Action: commented only
During development of the limited API support for PySide,
we saw an error in a macro that accessed a type field.

This patch fixes the 7 errors in the Python headers.
Macros which were not written as capitals were implemented
as function.

To do the necessary analysis again, a script was included that
parses all headers and looks for "->tp_" in sections which can
be reached with active limited API.

It is easily possible to call this script as a test.

Error listing:

../../Include/objimpl.h:243
    (Py_TYPE(o)->tp_is_gc == NULL || Py_TYPE(o)->tp_is_gc(o)))
Action: commented only

../../Include/objimpl.h:362
Action: commented only

../../Include/objimpl.h:364
    ((PyObject **) (((char *) (o)) + Py_TYPE(o)->tp_weaklistoffset))
Action: commented only

../../Include/pyerrors.h:143
     ((char *)(((PyTypeObject*)(x))->tp_name))
Action: implemented function

../../Include/abstract.h:593
    ((obj)->ob_type->tp_iternext != NULL && \
     (obj)->ob_type->tp_iternext != &_PyObject_NextNotImplemented)
Action: implemented function

../../Include/abstract.h:713
    ((obj)->ob_type->tp_as_number != NULL &&            \
     (obj)->ob_type->tp_as_number->nb_index != NULL)
Action: implemented function

../../Include/abstract.h:924
    ( Py_TYPE(o)->tp_as_sequence->sq_item(o, i) )
Action: commented only
@ctismer
Copy link
Copy Markdown
Contributor Author

ctismer commented Jun 11, 2018

@ned-deily Will the commit then arrive in 3.8 only, or earlier?

@ned-deily
Copy link
Copy Markdown
Member

@ctismer I think it is a desirable goal to get the stable ABI usable again. But I have no experience with it and I think that's true of most core developers. So I think we need to be cautious at this point about making sure that we do not inadvertently introduce incompatibilities that would affect the vast majority of our downstream users who do not use the stable ABI. At this point, I think the best thing to do is get things working in master (for 3.8) and then perhaps you and any others here (like possibly @serhiy-storchaka ) could come up with a proposal on python-dev for how we might proceed for 3.7.x and, possibly, 3.6.x, although it's probably too late in its cycle (e.g. only two more maint releases are planned after today's). I just am not comfortable potentially introducing a C level incompatibly, even new warnings, for 3.7.0 after we have promised our downstream users that the ABI was stable as of 3.7.0b2. The ideal should be that any extension that worked for 3.7.0b2 should work for any subsequent 3.7.x release. Sound OK?

@ctismer
Copy link
Copy Markdown
Contributor Author

ctismer commented Jun 12, 2018

@ned-deily Well, sounds ok! After the last change from @serhiy-storchaka I understand the thing now also completely from the python side, and I will try to become the shepherd for the limited API.
I will continue to work on the stuff and discuss on python-dev how to proceed.

BTW. We have successfully built PySide wheels for Python 3.5 - 3.7 and release the stuff right now.

macros PyIter_Check, PyIndex_Check and PyExceptionClass_Name were added as
functions. A script for automatic macro checks was added.
functions. The return type of PyExceptionClass_Name is "const char \*".
A script for automatic macro checks was added.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this script? I don't see it in this PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was added in the other PR. This PR is just post-commit cosmetic fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants