-
Notifications
You must be signed in to change notification settings - Fork 779
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
Improve native object initialization #4798
base: main
Are you sure you want to change the base?
Conversation
43ba51b
to
377feb8
Compare
@@ -830,12 +830,14 @@ impl<'a> FnSpec<'a> { | |||
_kwargs: *mut #pyo3_path::ffi::PyObject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume these are named with underscores because sometimes the generated code uses them and sometimes it doesn't?
) -> PyResult<*mut ffi::PyObject> { | ||
// HACK (due to FIXME below): PyBaseObject_Type's tp_new isn't happy with NULL arguments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there was a misunderstanding here, calling PyBaseObject_Type::tp_new
with an empty tuple and NULL kwargs is OK, but the implementation here was passing NULL for both, which is invalid.
@@ -1,9 +1,60 @@ | |||
//! Contains initialization utilities for `#[pyclass]`. | |||
//! | |||
//! # Background |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
although it seems fairly straightforward once compiled like this, it took quite a bit of experimentation to make sure I was getting things correct so I wanted to document my findings somewhere
377feb8
to
79ba367
Compare
let obj = tp_new( | ||
subtype, | ||
args.as_ptr(), | ||
kwargs | ||
.map(|obj| obj.as_ptr()) | ||
.unwrap_or(std::ptr::null_mut()), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a limitation here, where in python a class has to call super(MyClass, cls).__new__(cls, *args, **kwargs)
manually so it can choose which options to propagate to the super class. With the current approach the arguments propagate as-is
c5e2f75
to
9b946aa
Compare
Before this change, classes inheriting the base object was a special case where `object.__new__` is not called, and inheriting from other base classes requires use of the unlimited API. Previously this was not very limiting, but with <PyO3#4678> it will be possible to inherit from native base classes with the limited API and possibly from dynamically imported native base classes which may require `__new__` arguments to reach them.
9b946aa
to
57128c0
Compare
Before this change, classes inheriting the base object were constructed by using
TP_ALLOC
directly, obscuring the intention ofPyNativeTypeInitializer
, and inheriting from other base classes requires use of the unlimited API.Previously this was not very limiting, but with #4678 it will be possible to inherit from native base classes with the limited API and possibly from dynamically imported native base classes which may require
__new__
arguments to reach them.