Skip to content
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

ffi cleanup: object.h #1429

Merged
merged 8 commits into from
Feb 20, 2021
Merged

ffi cleanup: object.h #1429

merged 8 commits into from
Feb 20, 2021

Conversation

nw0
Copy link
Contributor

@nw0 nw0 commented Feb 14, 2021

This one's a fairly big change to account for the new cpython/object.h file.

Deprecate some FFI definitions which are not in the API

  • PyObject_Check
  • PySuper_Check
  • FreeFunc (note: there's also a freefunc which is subtly different -- fixed _PyEval_RequestCodeExtraIndex, which takes a freefunc as an argument)

I thought the INCREF, DECREF, ... methods were a bit off. See review (soon). EDIT: will open new issue this is fine

Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Looks great, thanks very much again. This is definitely one of the more awkward files; as always thank you for your diligent reconciliation.

src/ffi/object.rs Show resolved Hide resolved
Comment on lines 371 to 383
#[cfg(py_sys_config = "COUNT_ALLOCS")]
#[deprecated(note = "not present in Python headers; to be removed")]
pub const PyTypeObject_INIT: PyTypeObject = type_object_init! {
tp_allocs: 0,
tp_frees: 0,
tp_maxalloc: 0,
tp_prev: ptr::null_mut(),
tp_next: ptr::null_mut(),
};

#[cfg(not(py_sys_config = "COUNT_ALLOCS"))]
#[deprecated(note = "not present in Python headers; to be removed")]
pub const PyTypeObject_INIT: PyTypeObject = type_object_init!();
Copy link
Member

Choose a reason for hiding this comment

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

TBH I think this falls under the same bracket of the other _INIT constants which aren't actually defined upstream; we can just throw it away for the 0.14 release. Nobody should be using it.

Looks like that would allow quite a lot of macro code above to be deleted too!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought so, but didn't want to presume. Updated.

Shall we also remove PyObject_Check, PySuper_Check, and FreeFunc? They're not in the Python API either, and I struggle to imagine why, for example, a downstream user would need need a function to verify that a PyObject is a Python object (always returns 1).

Copy link
Member

Choose a reason for hiding this comment

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

Agreed that we can remove those three too!

Copy link
Member

@kngwyu kngwyu left a comment

Choose a reason for hiding this comment

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

Awesome 💯 , thank you.

@kngwyu kngwyu merged commit 2aeb021 into PyO3:master Feb 20, 2021
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.

3 participants