Merge pull request #783 from kngwyu/fix-pylayout

Fix PyLayout and restrict PyRef::as_ref for non-native types
2020-03-03 17:09:28 +09:00 · 2020-03-03 17:09:28 +09:00 · 3115667181
parent f470154cdf 68cfd5e17a
commit 3115667181
5 changed files with 67 additions and 54 deletions
--- a/guide/src/class.md
+++ b/guide/src/class.md
@ -83,7 +83,6 @@ impl pyo3::class::methods::PyMethodsInventoryDispatch for MyClass {
 }

 pyo3::inventory::collect!(MyClassGeneratedPyo3Inventory);
-
 # let gil = Python::acquire_gil();
 # let py = gil.python();
 # let cls = py.get_type::<MyClass>();
@ -249,7 +248,6 @@ or by `self_.into_super()` as `PyRef<Self::BaseClass>`.

 ```rust
 # use pyo3::prelude::*;
-use pyo3::PyCell;

 #[pyclass]
 struct BaseClass {
@ -305,18 +303,51 @@ impl SubSubClass {
      SubClass::method2(super_).map(|x| x * v)
   }
 }
-
-
 # let gil = Python::acquire_gil();
 # let py = gil.python();
 # let subsub = pyo3::PyCell::new(py, SubSubClass::new()).unwrap();
 # pyo3::py_run!(py, subsub, "assert subsub.method3() == 3000")
 ```
+You can also inherit native types such as `PyDict`, if it implements
+[`PySizedLayout`](https://pyo3.rs/master/doc/pyo3/type_object/trait.PySizedLayout.html).
+
+However, because of some technical problems, now we don't provide safe upcast methods for types
+that inherit native types. Even in such cases, you can get a base class by raw pointer conversion.
+
+```rust
+# use pyo3::prelude::*;
+use pyo3::types::{PyAny, PyDict};
+use pyo3::{AsPyPointer, PyNativeType};
+use std::collections::HashMap;
+
+#[pyclass(extends=PyDict)]
+#[derive(Default)]
+struct DictWithCounter {
+    counter: HashMap<String, usize>,
+}
+
+#[pymethods]
+impl DictWithCounter {
+   #[new]
+   fn new() -> Self {
+       Self::default()
+   }
+   fn set(mut self_: PyRefMut<Self>, key: String, value: &PyAny) -> PyResult<()> {
+       self_.counter.entry(key.clone()).or_insert(0);
+       let py = self_.py();
+       let dict: &PyDict = unsafe { py.from_borrowed_ptr_or_err(self_.as_ptr())? };
+       dict.set_item(key, value)
+   }
+}
+# let gil = Python::acquire_gil();
+# let py = gil.python();
+# let cnt = pyo3::PyCell::new(py, DictWithCounter::new()).unwrap();
+# pyo3::py_run!(py, cnt, "cnt.set('abc', 10); assert cnt['abc'] == 10")
+```

 If `SubClass` does not provide a baseclass initialization, the compilation fails.
 ```compile_fail
 # use pyo3::prelude::*;
-use pyo3::PyCell;

 #[pyclass]
 struct BaseClass {
@ -337,7 +368,6 @@ impl SubClass {
 }
 ```

-
 ## Object properties

 Property descriptor methods can be defined in a `#[pymethods]` `impl` block only and have to be
@ -345,14 +375,13 @@ annotated with `#[getter]` and `#[setter]` attributes. For example:

 ```rust
 # use pyo3::prelude::*;
-# #[pyclass]
-# struct MyClass {
-#    num: i32,
-# }
-#
+#[pyclass]
+struct MyClass {
+   num: i32,
+}
+
 #[pymethods]
 impl MyClass {
-
     #[getter]
     fn num(&self) -> PyResult<i32> {
        Ok(self.num)
@ -375,10 +404,8 @@ can be used since Rust 2018).
 # struct MyClass {
 #    num: i32,
 # }
-#
 #[pymethods]
 impl MyClass {
-
     #[getter]
     fn get_num(&self) -> PyResult<i32> {
        Ok(self.num)
@ -403,10 +430,8 @@ If this parameter is specified, it is used as the property name, i.e.
 # struct MyClass {
 #    num: i32,
 # }
-#
 #[pymethods]
 impl MyClass {
-
     #[getter(number)]
     fn num(&self) -> PyResult<i32> {
        Ok(self.num)
@ -448,10 +473,8 @@ block with some variations, like descriptors, class method static methods, etc.
 # struct MyClass {
 #    num: i32,
 # }
-#
 #[pymethods]
 impl MyClass {
-
     fn method1(&self) -> PyResult<i32> {
        Ok(10)
     }
@ -477,7 +500,6 @@ gets injected by the method wrapper, e.g.
 #    num: i32,
 #    debug: bool,
 # }
-
 #[pymethods]
 impl MyClass {
     fn method2(&self, py: Python) -> PyResult<i32> {
@ -501,7 +523,6 @@ with the `#[classmethod]` attribute.
 #    num: i32,
 #    debug: bool,
 # }
-
 #[pymethods]
 impl MyClass {
     #[classmethod]
@ -532,7 +553,6 @@ To create a static method for a custom class, the method needs to be annotated w
 #    num: i32,
 #    debug: bool,
 # }
-
 #[pymethods]
 impl MyClass {
     #[staticmethod]
@ -555,7 +575,6 @@ use pyo3::types::PyTuple;
 #    num: i32,
 #    debug: bool,
 # }
-
 #[pymethods]
 impl MyClass {
     #[call]
@ -598,7 +617,6 @@ use pyo3::types::{PyDict, PyTuple};
 #    num: i32,
 #    debug: bool,
 # }
-#
 #[pymethods]
 impl MyClass {
    #[new]
@ -785,7 +803,7 @@ Example:

 ```rust
 use pyo3::prelude::*;
-use pyo3::{PyIterProtocol, PyCell};
+use pyo3::PyIterProtocol;

 #[pyclass]
 struct MyIterator {
--- a/src/pycell.rs
+++ b/src/pycell.rs
@ -25,9 +25,6 @@ where
    T::Layout: PySizedLayout<T>,
 {
    const IS_NATIVE_TYPE: bool = true;
-    unsafe fn get_ptr(&self) -> *mut T {
-        (&self) as *const &Self as *const _ as *mut _
-    }
 }

 // Thes impls ensures `PyCellBase` can be a base type.
@ -67,9 +64,6 @@ unsafe impl<T: PyClass> PyLayout<T> for PyCellInner<T> {
    fn get_super(&mut self) -> Option<&mut T::BaseLayout> {
        Some(&mut self.ob_base)
    }
-    unsafe fn get_ptr(&self) -> *mut T {
-        self.value.get()
-    }
    unsafe fn py_init(&mut self, value: T) {
        self.value = ManuallyDrop::new(UnsafeCell::new(value));
    }
@ -84,6 +78,9 @@ impl<T: PyClass> PySizedLayout<T> for PyCellInner<T> {}
 unsafe impl<T: PyClass> PyBorrowFlagLayout<T> for PyCellInner<T> {}

 impl<T: PyClass> PyCellInner<T> {
+    unsafe fn get_ptr(&self) -> *mut T {
+        self.value.get()
+    }
    fn get_borrow_flag(&self) -> BorrowFlag {
        let base = (&self.ob_base) as *const _ as *const PyCellBase<T::BaseNativeType>;
        unsafe { (*base).borrow_flag.get() }
@ -353,9 +350,6 @@ unsafe impl<T: PyClass> PyLayout<T> for PyCell<T> {
    fn get_super(&mut self) -> Option<&mut T::BaseLayout> {
        Some(&mut self.inner.ob_base)
    }
-    unsafe fn get_ptr(&self) -> *mut T {
-        self.inner.get_ptr()
-    }
    unsafe fn py_init(&mut self, value: T) {
        self.inner.value = ManuallyDrop::new(UnsafeCell::new(value));
    }
@ -446,7 +440,13 @@ pub struct PyRef<'p, T: PyClass> {
    inner: &'p PyCellInner<T>,
 }

-impl<'p, T: PyClass> AsRef<T::BaseType> for PyRef<'p, T> {
+unsafe impl<'p, T: PyClass> crate::PyNativeType for PyRef<'p, T> {}
+
+impl<'p, T, U> AsRef<U> for PyRef<'p, T>
+where
+    T: PyClass + PyTypeInfo<BaseType = U, BaseLayout = PyCellInner<U>>,
+    U: PyClass,
+{
    fn as_ref(&self) -> &T::BaseType {
        unsafe { &*self.inner.ob_base.get_ptr() }
    }
@ -551,13 +551,23 @@ pub struct PyRefMut<'p, T: PyClass> {
    inner: &'p PyCellInner<T>,
 }

-impl<'p, T: PyClass> AsRef<T::BaseType> for PyRefMut<'p, T> {
+unsafe impl<'p, T: PyClass> crate::PyNativeType for PyRefMut<'p, T> {}
+
+impl<'p, T, U> AsRef<U> for PyRefMut<'p, T>
+where
+    T: PyClass + PyTypeInfo<BaseType = U, BaseLayout = PyCellInner<U>>,
+    U: PyClass,
+{
    fn as_ref(&self) -> &T::BaseType {
        unsafe { &*self.inner.ob_base.get_ptr() }
    }
 }

-impl<'p, T: PyClass> AsMut<T::BaseType> for PyRefMut<'p, T> {
+impl<'p, T, U> AsMut<U> for PyRefMut<'p, T>
+where
+    T: PyClass + PyTypeInfo<BaseType = U, BaseLayout = PyCellInner<U>>,
+    U: PyClass,
+{
    fn as_mut(&mut self) -> &mut T::BaseType {
        unsafe { &mut *self.inner.ob_base.get_ptr() }
    }
--- a/src/type_object.rs
+++ b/src/type_object.rs
@ -21,7 +21,6 @@ pub unsafe trait PyLayout<T: PyTypeInfo> {
    }
    unsafe fn py_init(&mut self, _value: T) {}
    unsafe fn py_drop(&mut self, _py: Python) {}
-    unsafe fn get_ptr(&self) -> *mut T;
 }

 /// `T: PySizedLayout<U>` represents `T` is not a instance of
--- a/src/types/any.rs
+++ b/src/types/any.rs
@ -23,11 +23,7 @@ use crate::{ffi, PyObject};
 /// ```
 #[repr(transparent)]
 pub struct PyAny(PyObject, Unsendable);
-unsafe impl crate::type_object::PyLayout<PyAny> for ffi::PyObject {
-    unsafe fn get_ptr(&self) -> *mut PyAny {
-        (&self) as *const &Self as *const _ as *mut _
-    }
-}
+unsafe impl crate::type_object::PyLayout<PyAny> for ffi::PyObject {}
 impl crate::type_object::PySizedLayout<PyAny> for ffi::PyObject {}
 pyobject_native_type_named!(PyAny);
 pyobject_native_type_convert!(
--- a/src/types/mod.rs
+++ b/src/types/mod.rs
@ -56,20 +56,10 @@ macro_rules! pyobject_native_type_named (
    };
 );

-macro_rules! impl_layout {
-    ($name: ty, $layout: path) => {
-        unsafe impl $crate::type_object::PyLayout<$name> for $layout {
-            unsafe fn get_ptr(&self) -> *mut $name {
-                (&self) as *const &Self as *const _ as *mut _
-            }
-        }
-    };
-}
-
 #[macro_export]
 macro_rules! pyobject_native_type {
    ($name: ty, $layout: path, $typeobject: expr, $module: expr, $checkfunction: path $(,$type_param: ident)*) => {
-        impl_layout!($name, $layout);
+        unsafe impl $crate::type_object::PyLayout<$name> for $layout {}
        impl $crate::type_object::PySizedLayout<$name> for $layout {}
        impl $crate::derive_utils::PyBaseTypeUtils for $name {
            type Dict = $crate::pyclass_slots::PyClassDummySlot;
@ -97,7 +87,7 @@ macro_rules! pyobject_native_type {
 #[macro_export]
 macro_rules! pyobject_native_var_type {
    ($name: ty, $typeobject: expr, $module: expr, $checkfunction: path $(,$type_param: ident)*) => {
-        impl_layout!($name, $crate::ffi::PyObject);
+        unsafe impl $crate::type_object::PyLayout<$name> for $crate::ffi::PyObject {}
        pyobject_native_type_named!($name $(,$type_param)*);
        pyobject_native_type_convert!($name, $crate::ffi::PyObject,
                                      $typeobject, $module, $checkfunction $(,$type_param)*);