Skip to content

bpo-35081: Make some _PyGC macros internal #10507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 13, 2018
Merged

bpo-35081: Make some _PyGC macros internal #10507

merged 1 commit into from
Nov 13, 2018

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Nov 13, 2018

  • Move "GC" macros together:

    • PyObject_IS_GC()
    • _Py_AS_GC()
    • _PyObject_GC_IS_TRACKED()
    • _PyObject_GC_MAY_BE_TRACKED()
  • Mark other GC macros as internal (#ifdef Py_BUILD_CORE)

  • Replace _PyGC_generation0 with _PyRuntime.gc.generation0

  • _queuemodule.c: replace _PyObject_GC_UNTRACK()
    with with PyObject_GC_Track()

https://bugs.python.org/issue35081

@vstinner
Copy link
Member Author

I'm not really satisfied by my previous attempts to "Move _PyObject_GC_TRACK() to pycore_object.h": PR #10272 and PR #10276, so I created this PR to "prepare the code" for the second change.

@methane
Copy link
Member

methane commented Nov 13, 2018

  • _PyObject_GC_UNTRACK()
    with with PyObject_GC_Track()

UNTRACK and Track are opposite.

@vstinner
Copy link
Member Author

UNTRACK and Track are opposite.

Ooooops :-) I fixed my PR.

I just noticed that _PyObject_GC_TRACK() _PyObject_GC_UNTRACK() macros are documented as "should not be used for extension modules". If I understand correctly, they macros have been broken in Python 3.7 since _PyObject_GC_TRACK() now requires Include/internal/pycore_pystate.h (called Include/internal/pystate.h in Python 3.7) to access _PyRuntime.gc.generation0. Include/internal/ headers are not installed by "make install", and so not usable by 3rd party extensions.

I understand that my PR is not really backward incompatible, since the macros have been broken since Python 3.7 already. Moreover, they were never intended to be used outside CPython... right?

* Move "GC" macros together:

  * PyObject_IS_GC()
  * _Py_AS_GC()
  * _PyObject_GC_IS_TRACKED()
  * _PyObject_GC_MAY_BE_TRACKED()

* Mark other GC macros as internal (#ifdef Py_BUILD_CORE):

  * _PyGCHead_NEXT(g), _PyGCHead_SET_NEXT(g, p)
  * _PyGCHead_PREV(g), _PyGCHead_SET_PREV(g, p)
  * _PyGCHead_FINALIZED(g), _PyGCHead_SET_FINALIZED(g)
  * _PyGC_FINALIZED(o), _PyGC_SET_FINALIZED(o)
  * _PyObject_GC_TRACK(o), _PyObject_GC_UNTRACK(o)
  * _PyGC_PREV_MASK_FINALIZED
  * _PyGC_PREV_MASK_COLLECTING
  * _PyGC_PREV_SHIFT
  * _PyGC_PREV_MASK

* Replace _PyGC_generation0 with _PyRuntime.gc.generation0
* _queuemodule.c: replace _PyObject_GC_UNTRACK()
  with with PyObject_GC_UnTrack()
* Document that  _PyObject_GC_TRACK() _PyObject_GC_UNTRACK() macros
  have been removed from the public C API.
@vstinner
Copy link
Member Author

I used git push --force a second time to include the full list of macros removed from the public API in the commit message:

  • _PyGCHead_NEXT(g), _PyGCHead_SET_NEXT(g, p)
  • _PyGCHead_PREV(g), _PyGCHead_SET_PREV(g, p)
  • _PyGCHead_FINALIZED(g), _PyGCHead_SET_FINALIZED(g)
  • _PyGC_FINALIZED(o), _PyGC_SET_FINALIZED(o)
  • _PyObject_GC_TRACK(o), _PyObject_GC_UNTRACK(o)
  • _PyGC_PREV_MASK_FINALIZED
  • _PyGC_PREV_MASK_COLLECTING
  • _PyGC_PREV_SHIFT
  • _PyGC_PREV_MASK


#define _PyGC_FINALIZED(o) _PyGCHead_FINALIZED(_Py_AS_GC(o))
#define _PyGC_SET_FINALIZED(o) _PyGCHead_SET_FINALIZED(_Py_AS_GC(o))
#define _PyGC_FINALIZED(o) \
Copy link
Contributor

Choose a reason for hiding this comment

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

Making this internal broke Cython (cython/cython#2721). It's used in the finaliser implementation, to determine if an object for which tp_dealloc() is called has already been finalised or whether we have to do it. I'm not sure how to deal with this on our side now. Any clue?

@etejedor
Copy link

etejedor commented Dec 4, 2018

Will this be ported to 3.7 too? The _PyObject_GC_TRACK macro is broken in 3.7 too, which makes e.g. PyCFunction_New crash. Thank you!

@vstinner
Copy link
Member Author

vstinner commented Dec 4, 2018

Will this be ported to 3.7 too? The _PyObject_GC_TRACK macro is broken in 3.7 too, which makes e.g. PyCFunction_New crash. Thank you!

I'm not aware of this issue. Please open a new bug report at https://bugs.python.org/

@etejedor
Copy link

etejedor commented Dec 4, 2018

Thanks for the quick reply @vstinner . Sure, I will open a new bug report.

On the other hand, I found that you already mention here:

https://bugs.python.org/issue35229

that the _PyObject_GC_TRACK() and _PyObject_GC_UNTRACK() have been broken in 3.7 and you explain the reason.

@vstinner
Copy link
Member Author

vstinner commented Dec 4, 2018

which makes e.g. PyCFunction_New crash

I'm not aware of that. Does it crash at runtime? In a module part of the stdlib or in a third-party C extension? If it's third-party, how is the C extension compiled? Well, please explain all these stuff in your bug report :-)

I deprecated the functions in Python 3.6 doc:
60a6bae

@etejedor
Copy link

etejedor commented Dec 4, 2018

@vstinner please find here the details:
https://bugs.python.org/issue35408

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

Successfully merging this pull request may close these issues.

6 participants