◐ Shell
reader mode source ↗
Skip to content

bpo-35081: Make some _PyGC macros internal#10507

Merged
vstinner merged 1 commit into
python:masterfrom
vstinner:gchead
Nov 13, 2018
Merged

bpo-35081: Make some _PyGC macros internal#10507
vstinner merged 1 commit into
python:masterfrom
vstinner:gchead

Conversation

@vstinner

@vstinner vstinner commented Nov 13, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

  • 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 vstinner requested a review from methane November 13, 2018 08:52
@vstinner vstinner requested a review from rhettinger as a code owner November 13, 2018 08:52
@vstinner

vstinner commented Nov 13, 2018

Copy link
Copy Markdown
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

methane commented Nov 13, 2018

Copy link
Copy Markdown
Member
  • _PyObject_GC_UNTRACK()
    with with PyObject_GC_Track()

UNTRACK and Track are opposite.

@vstinner

vstinner commented Nov 13, 2018

Copy link
Copy Markdown
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?

@vstinner
bpo-35081: Make some _PyGC macros internal …
6e8a93b 
* 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

vstinner commented Nov 13, 2018

Copy link
Copy Markdown
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

@vstinner vstinner merged commit 1a6be91 into python:master Nov 13, 2018
@vstinner vstinner deleted the gchead branch November 13, 2018 11:52
@vstinner vstinner mentioned this pull request Nov 13, 2018
Closed
@etejedor

etejedor commented Dec 4, 2018

Copy link
Copy Markdown

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

vstinner commented Dec 4, 2018

Copy link
Copy Markdown
Member Author

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

etejedor commented Dec 4, 2018

Copy link
Copy Markdown

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

vstinner commented Dec 4, 2018

Copy link
Copy Markdown
Member Author

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

etejedor commented Dec 4, 2018

Copy link
Copy Markdown

@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

Reviewers

@scoder scoder

@methane methane methane approved these changes

@rhettinger rhettinger Awaiting requested review from rhettinger

@serhiy-storchaka serhiy-storchaka Awaiting requested review from serhiy-storchaka

@pitrou pitrou Awaiting requested review from pitrou

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@vstinner @methane @etejedor @scoder @the-knights-who-say-ni @bedevere-bot