Change the initializer style

CHANGELOG.md

@@ -1,4 +1,5 @@
 # Changelog
+
 All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
@@ -10,22 +11,31 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
  * Added a `wrap_pymodule!` macro similar to the existing `wrap_pyfunction!` macro. Only available on python 3
  * Added support for cross compiling (e.g. to arm v7) by mtp401 in [#327](https://github.com/PyO3/pyo3/pull/327). See the "Cross Compiling" section in the "Building and Distribution" chapter of the guide for more details.
+ * The `PyRef` and `PyRefMut` types, which allow to differentiate between an instance of a rust struct on the rust heap and an instance that is embedded inside a python object. By kngwyu in [#335](https://github.com/PyO3/pyo3/pull/335)
 
 ### Changed
 
  * Renamed `add_function` to `add_wrapped` as it now also supports modules.
- * Renamed `#[pymodinit]` to `#[pymodule]`.
- * `init`, `init_ref` and `init_mut` now take a value instead of a function that returns the value. (Migrating normally just removing `||`)
- * Renamed `py_exception` to `create_exception` and refactored the error macros.
+ * Renamed `#[pymodinit]` to `#[pymodule]`
+ * `py.init(|| value)` becomes `Py::new(value)`
+ * `py.init_ref(|| value)` becomes `PyRef::new(value)`
+ * `py.init_mut(|| value)` becomes `PyRefMut::new(value)`.
+ * `PyRawObject::init` is now infallible, e.g. it returns `()` instead of `PyResult<()>`.
+ * Renamed `py_exception!` to `create_exception!` and refactored the error macros.
  * Renamed `wrap_function!` to `wrap_pyfunction!`
  * Migrated to the 2018 edition
  * Replace `IntoPyTuple` with `IntoPy<Py<PyTuple>>`. Eventually `IntoPy<T>` should replace `ToPyObject` and be itself implemented through `FromPy<T>`
+ * PyTypeObject is now a direct subtrait PyTypeCreate, removing the old cyclical implementation in [#350](https://github.com/PyO3/pyo3/pull/350)
 
 ### Removed
 
  * `PyToken` was removed due to unsoundness (See [#94](https://github.com/PyO3/pyo3/issues/94)).
  * Removed the unnecessary type parameter from `PyObjectAlloc`
 
+### Fixed
+
+ * A soudness hole where every instances of a `#[pyclass]` struct was considered to be part of a python object, even though you can create instances that are not part of the python heap. This was fixed through `PyRef` and `PyRefMut`. 
+
 ## [0.5.3] - 2019-01-04
 
 ### Fixed

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "pyo3"
-version = "0.6.0-alpha.2"
+version = "0.6.0-alpha.3"
 description = "Bindings to Python interpreter"
 authors = ["PyO3 Project and Contributors <https://github.com/PyO3>"]
 readme = "README.md"

examples/rustapi_module/src/datetime.rs

@@ -191,7 +191,7 @@ pub struct TzClass {}
 impl TzClass {
     #[new]
     fn __new__(obj: &PyRawObject) -> PyResult<()> {
-        obj.init(TzClass {})
+        Ok(obj.init(TzClass {}))
     }
 
     fn utcoffset(&self, py: Python<'_>, _dt: &PyDateTime) -> PyResult<Py<PyDelta>> {

examples/rustapi_module/src/dict_iter.rs

@@ -17,7 +17,7 @@ pub struct DictSize {
 impl DictSize {
     #[new]
     fn __new__(obj: &PyRawObject, expected: u32) -> PyResult<()> {
-        obj.init(DictSize { expected })
+        Ok(obj.init(DictSize { expected }))
     }
 
     fn iter_dict(&mut self, _py: Python<'_>, dict: &PyDict) -> PyResult<u32> {

examples/rustapi_module/src/othermod.rs

@@ -14,9 +14,9 @@ pub struct ModClass {
 impl ModClass {
     #[new]
     fn __new__(obj: &PyRawObject) -> PyResult<()> {
-        obj.init(ModClass {
+        Ok(obj.init(ModClass {
             _somefield: String::from("contents"),
-        })
+        }))
     }
 
     fn noop(&self, x: usize) -> usize {

examples/rustapi_module/src/subclassing.rs

@@ -9,7 +9,7 @@ pub struct Subclassable {}
 impl Subclassable {
     #[new]
     fn __new__(obj: &PyRawObject) -> PyResult<()> {
-        obj.init(Subclassable {})
+        Ok(obj.init(Subclassable {}))
     }
 }
 

examples/word-count/src/lib.rs

@@ -17,9 +17,9 @@ struct WordCounter {
 impl WordCounter {
     #[new]
     fn __new__(obj: &PyRawObject, path: String) -> PyResult<()> {
-        obj.init(WordCounter {
+        Ok(obj.init(WordCounter {
             path: PathBuf::from(path),
-        })
+        }))
     }
 
     /// Searches for the word, parallelized by rayon

src/freelist.rs

@@ -2,7 +2,6 @@
 
 //! Free allocation list
 
-use crate::err::PyResult;
 use crate::ffi;
 use crate::python::Python;
 use crate::typeob::{pytype_drop, PyObjectAlloc, PyTypeInfo};
@@ -72,15 +71,13 @@ impl<T> PyObjectAlloc for T
 where
     T: PyObjectWithFreeList,
 {
-    unsafe fn alloc(_py: Python) -> PyResult<*mut ffi::PyObject> {
-        let obj = if let Some(obj) = <Self as PyObjectWithFreeList>::get_free_list().pop() {
+    unsafe fn alloc(_py: Python) -> *mut ffi::PyObject {
+        if let Some(obj) = <Self as PyObjectWithFreeList>::get_free_list().pop() {
             ffi::PyObject_Init(obj, <Self as PyTypeInfo>::type_object());
             obj
         } else {
             ffi::PyType_GenericAlloc(<Self as PyTypeInfo>::type_object(), 0)
-        };
-
-        Ok(obj)
+        }
     }
 
     #[cfg(Py_3)]

src/instance.rs

@@ -71,7 +71,7 @@ where
 {
     pub fn new(py: Python, value: T) -> PyResult<PyRef<T>> {
         let obj = T::create(py)?;
-        obj.init(value)?;
+        obj.init(value);
         let ref_ = unsafe { py.from_owned_ptr(obj.into_ptr()) };
         Ok(PyRef::from_ref(ref_))
     }
@@ -134,7 +134,7 @@ where
 {
     pub fn new(py: Python, value: T) -> PyResult<PyRefMut<T>> {
         let obj = T::create(py)?;
-        obj.init(value)?;
+        obj.init(value);
         let ref_ = unsafe { py.mut_from_owned_ptr(obj.into_ptr()) };
         Ok(PyRefMut::from_mut(ref_))
     }
@@ -257,15 +257,30 @@ pub trait AsPyRef<T: PyTypeInfo>: Sized {
 }
 
 /// Safe wrapper around unsafe `*mut ffi::PyObject` pointer with specified type information.
+///
+/// `Py<T>` is thread-safe, because any python related operations require a Python<'p> token.
 #[derive(Debug)]
 #[repr(transparent)]
 pub struct Py<T>(NonNull<ffi::PyObject>, std::marker::PhantomData<T>);
 
-// `Py<T>` is thread-safe, because any python related operations require a Python<'p> token.
 unsafe impl<T> Send for Py<T> {}
 
 unsafe impl<T> Sync for Py<T> {}
 
+impl<T> Py<T>
+where
+    T: PyTypeCreate + PyTypeObject,
+{
+    /// Create new instance of T and move it under python management
+    pub fn new(py: Python, value: T) -> PyResult<Py<T>> {
+        let ob = T::create(py)?;
+        ob.init(value);
+
+        let ob = unsafe { Py::from_owned_ptr(ob.into_ptr()) };
+        Ok(ob)
+    }
+}
+
 impl<T> Py<T> {
     /// Creates a `Py<T>` instance for the given FFI pointer.
     /// This moves ownership over the pointer into the `Py<T>`.
@@ -338,48 +353,6 @@ impl<T> Py<T> {
     }
 }
 
-impl<T> Py<T>
-where
-    T: PyTypeCreate,
-{
-    /// Create new instance of T and move it under python management
-    /// Returns `Py<T>`.
-    pub fn new(py: Python, value: T) -> PyResult<Py<T>>
-    where
-        T: PyTypeObject + PyTypeInfo,
-    {
-        let ob = <T as PyTypeCreate>::create(py)?;
-        ob.init(value)?;
-
-        let ob = unsafe { Py::from_owned_ptr(ob.into_ptr()) };
-        Ok(ob)
-    }
-
-    /// Create new instance of `T` and move it under python management.
-    /// Returns references to `T`
-    pub fn new_ref(py: Python, value: T) -> PyResult<&T>
-    where
-        T: PyTypeObject + PyTypeInfo,
-    {
-        let ob = <T as PyTypeCreate>::create(py)?;
-        ob.init(value)?;
-
-        unsafe { Ok(py.from_owned_ptr(ob.into_ptr())) }
-    }
-
-    /// Create new instance of `T` and move it under python management.
-    /// Returns mutable references to `T`
-    pub fn new_mut(py: Python, value: T) -> PyResult<&mut T>
-    where
-        T: PyTypeObject + PyTypeInfo,
-    {
-        let ob = <T as PyTypeCreate>::create(py)?;
-        ob.init(value)?;
-
-        unsafe { Ok(py.mut_from_owned_ptr(ob.into_ptr())) }
-    }
-}
-
 /// Specialization workaround
 trait AsPyRefDispatch<T: PyTypeInfo>: ToPyPointer {
     fn as_ref_dispatch(&self, _py: Python) -> &T {

src/typeob.rs

@@ -83,7 +83,7 @@ pub const PY_TYPE_FLAG_DICT: usize = 1 << 3;
 /// impl MyClass {
 ///    #[new]
 ///    fn __new__(obj: &PyRawObject) -> PyResult<()> {
-///        obj.init(MyClass { })
+///        Ok(obj.init(MyClass { }))
 ///    }
 /// }
 /// ```
@@ -139,16 +139,12 @@ impl PyRawObject {
         }
     }
 
-    pub fn init<T>(&self, value: T) -> PyResult<()>
-    where
-        T: PyTypeInfo,
-    {
+    pub fn init<T: PyTypeInfo>(&self, value: T) {
         unsafe {
             // The `as *mut u8` part is required because the offset is in bytes
             let ptr = (self.ptr as *mut u8).offset(T::OFFSET) as *mut T;
             std::ptr::write(ptr, value);
         }
-        Ok(())
     }
 
     /// Type object
@@ -195,12 +191,10 @@ pub(crate) unsafe fn pytype_drop<T: PyTypeInfo>(py: Python, obj: *mut ffi::PyObj
 /// All native types and all `#[pyclass]` types use the default functions, while
 /// [PyObjectWithFreeList](crate::freelist::PyObjectWithFreeList) gets a special version.
 pub trait PyObjectAlloc: PyTypeInfo + Sized {
-    unsafe fn alloc(_py: Python) -> PyResult<*mut ffi::PyObject> {
+    unsafe fn alloc(_py: Python) -> *mut ffi::PyObject {
         let tp_ptr = Self::type_object();
         let alloc = (*tp_ptr).tp_alloc.unwrap_or(ffi::PyType_GenericAlloc);
-        let obj = alloc(tp_ptr, 0);
-
-        Ok(obj)
+        alloc(tp_ptr, 0)
     }
 
     /// Calls the rust destructor for the object and frees the memory
@@ -286,7 +280,7 @@ pub trait PyTypeCreate: PyObjectAlloc + PyTypeObject + Sized {
         Self::init_type();
 
         unsafe {
-            let ptr = Self::alloc(py)?;
+            let ptr = Self::alloc(py);
             PyRawObject::new_with_ptr(
                 py,
                 ptr,

tests/test_arithmetics.rs

@@ -32,7 +32,7 @@ fn unary_arithmetic() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let c = py.init(UnaryArithmetic {}).unwrap();
+    let c = Py::new(py, UnaryArithmetic {}).unwrap();
     py_run!(py, c, "assert -c == 'neg'");
     py_run!(py, c, "assert +c == 'pos'");
     py_run!(py, c, "assert abs(c) == 'abs'");
@@ -110,7 +110,7 @@ fn inplace_operations() {
     let py = gil.python();
 
     let init = |value, code| {
-        let c = py.init(InPlaceOperations { value }).unwrap();
+        let c = Py::new(py, InPlaceOperations { value }).unwrap();
         py_run!(py, c, code);
     };
 
@@ -164,7 +164,7 @@ fn binary_arithmetic() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let c = py.init(BinaryArithmetic {}).unwrap();
+    let c = Py::new(py, BinaryArithmetic {}).unwrap();
     py_run!(py, c, "assert c + c == 'BA + BA'");
     py_run!(py, c, "assert c + 1 == 'BA + 1'");
     py_run!(py, c, "assert 1 + c == '1 + BA'");
@@ -230,7 +230,7 @@ fn rich_comparisons() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let c = py.init(RichComparisons {}).unwrap();
+    let c = Py::new(py, RichComparisons {}).unwrap();
     py_run!(py, c, "assert (c < c) == 'RC < RC'");
     py_run!(py, c, "assert (c < 1) == 'RC < 1'");
     py_run!(py, c, "assert (1 < c) == 'RC > 1'");
@@ -257,7 +257,7 @@ fn rich_comparisons_python_3_type_error() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let c2 = py.init(RichComparisons2 {}).unwrap();
+    let c2 = Py::new(py, RichComparisons2 {}).unwrap();
     py_expect_exception!(py, c2, "c2 < c2", TypeError);
     py_expect_exception!(py, c2, "c2 < 1", TypeError);
     py_expect_exception!(py, c2, "1 < c2", TypeError);

tests/test_buffer_protocol.rs

@@ -65,11 +65,13 @@ fn test_buffer() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let t = py
-        .init(TestClass {
+    let t = Py::new(
+        py,
+        TestClass {
             vec: vec![b' ', b'2', b'3'],
-        })
-        .unwrap();
+        },
+    )
+    .unwrap();
 
     let d = PyDict::new(py);
     d.set_item("ob", t).unwrap();
@@ -82,11 +84,13 @@ fn test_buffer() {
     let gil = Python::acquire_gil();
     let py = gil.python();
 
-    let t = py
-        .init(TestClass {
+    let t = Py::new(
+        py,
+        TestClass {
             vec: vec![b' ', b'2', b'3'],
-        })
-        .unwrap();
+        },
+    )
+    .unwrap();
 
     let d = PyDict::new(py);
     d.set_item("ob", t).unwrap();

tests/test_class_new.rs

@@ -8,7 +8,7 @@ struct EmptyClassWithNew {}
 impl EmptyClassWithNew {
     #[__new__]
     fn __new__(obj: &PyRawObject) -> PyResult<()> {
-        obj.init(EmptyClassWithNew {})
+        Ok(obj.init(EmptyClassWithNew {}))
     }
 }
 
@@ -33,7 +33,7 @@ struct NewWithOneArg {
 impl NewWithOneArg {
     #[new]
     fn __new__(obj: &PyRawObject, arg: i32) -> PyResult<()> {
-        obj.init(NewWithOneArg { _data: arg })
+        Ok(obj.init(NewWithOneArg { _data: arg }))
     }
 }
 
@@ -57,10 +57,10 @@ struct NewWithTwoArgs {
 impl NewWithTwoArgs {
     #[new]
     fn __new__(obj: &PyRawObject, arg1: i32, arg2: i32) -> PyResult<()> {
-        obj.init(NewWithTwoArgs {
+        Ok(obj.init(NewWithTwoArgs {
             _data1: arg1,
             _data2: arg2,
-        })
+        }))
     }
 }