ffi module cleanup: context.h to frameobject.h #1341
Conversation
There was a problem hiding this comment.
Thanks again for continuing on with this, this is looking excellent! I see a few TODO comments so I'll give this another look through once this is resolved.
Regarding the CHANGELOG - we usually make the sections Added, Changed, Removed, Fixed in that order, so if you're adding a new section would be great to make it fit that ordering.
|
|
||
| /// Helper initial value of [`PyGetSetDef`] for a Python class. | ||
| /// | ||
| /// Not present in `cpython/Include/descrobject`. |
There was a problem hiding this comment.
By this, you mean it's something we have invented ourselves?
There was a problem hiding this comment.
Yes, I don't find any obvious origin in cpython. If ffi is strictly for imports, we should probably move this, but it's up to you.
There was a problem hiding this comment.
Yeah, I think this (and PyGetSetDef_DICT) should probably be moved to just be private variables in pyclass.rs.
For now please just mark these deprecated; I'll move them in a future breaking release.
There was a problem hiding this comment.
I see a few TODO comments so I'll give this another look through once this is resolved.
I've annotated these. Basically, these can't be moved as-is (or mark them non-limited), because we have code in dict.rs and floatob.rs (py_native_type invocations) that unconditionally uses these types. Could you advise on the appropriate action here?
|
I think the right solution for Line 44 in 3f093d9 |
CHANGELOG.md
Outdated
| ### Removed | ||
| - Remove FFI definition `PyImport_cleanup` when building for Python 3.9 or later, to mirror Python headers. [#1341](https://github.com/PyO3/pyo3/pull/1341) | ||
| - Remove FFI definition `PyOS_InitInterrupts` when building for Python 3.10 or later, to mirror Python headers. [#1341](https://github.com/PyO3/pyo3/pull/1341) |
There was a problem hiding this comment.
I think no need for these points in Removed because you've already mentioned the respective functions in Changed.
CHANGELOG.md
Outdated
| @@ -6,10 +6,21 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) | |||
| and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). | |||
|
|
|||
| ## [Unreleased] | |||
| ### Changed | |||
| - Deprecate FFI definition `PyCoro_Check` and `PyAsyncGen_Check` in favor of `PyCoro_CheckExact` and `PyAsyncGen_CheckExact` respectively, as these are the correct name in the Python headers. [#1341](https://github.com/PyO3/pyo3/pull/1341) | |||
| - Deprecate FFI definition `PyCoroWrapper_Check` and `PyGetSetDef_DICT` which have never been defined in the Python headers. [#1341](https://github.com/PyO3/pyo3/pull/1341) | |||
There was a problem hiding this comment.
Maybe also PyGetSetDef_INIT?
CHANGELOG.md
Outdated
| - Deprecate FFI definition `PyCoroWrapper_Check` and `PyGetSetDef_DICT` which have never been defined in the Python headers. [#1341](https://github.com/PyO3/pyo3/pull/1341) | ||
| - Deprecate FFI definition `PyImport_Cleanup`, which was removed in 3.9 and previously marked "for internal use only". [#1341](https://github.com/PyO3/pyo3/pull/1341) | ||
| - Deprecate FFI definition `PyOS_InitInterrupts`, which was removed in 3.10 and previously undocumented. [#1341](https://github.com/PyO3/pyo3/pull/1341) | ||
| - Deprecate FFI definition `PyOS_AfterFork`; introduce `PyOS_BeforeFork`, `PyOS_AfterFork_Parent`, `PyOS_AfterFork_Child` when building for Python 3.7. [#1341](https://github.com/PyO3/pyo3/pull/1341) |
There was a problem hiding this comment.
The new methods should be in the Added section please :)
src/ffi/cpython/dictobject.rs
Outdated
| // skipped _PyDictViewObject | ||
| // skipped _PyDictView_New | ||
| // skipped _PyDictView_Intersect | ||
| // FIXME: PyDict_Contains is defined in dictobject.c |
There was a problem hiding this comment.
Oops -- I was in the middle of tracking this down, will update. Basically it's removed in 3.10, but not in the Python changelog (yet).
src/ffi/cpython/frameobject.rs
Outdated
| // skipped _PyFrame_DebugMallocStats | ||
| // skipped PyFrame_GetBack | ||
|
|
||
| // FIXME: PyFrame_ClearFreeList is defined in frameobject.c |
There was a problem hiding this comment.
Similarly not sure what the FIXME is asking us to do?
Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
|
Sorry about the mess. I've eyeballed the PR diff this time and think it's ready for review, if tests pass. |
Partial fix for #1289. This set seems relatively more straightforward, but touches a lot of files.