Merge pull request #260 from ijl/nonnull

NonNull pointer for Py, PyObject
PyO3 · Nov 11, 2018 · ef68b22 · ef68b22
2 parents ecae854 + 50c6129
commit ef68b22
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 42 deletions.
diff --git a/src/instance.rs b/src/instance.rs
@@ -2,6 +2,7 @@
 
 use std;
 use std::marker::PhantomData;
+use std::ptr::NonNull;
 use std::rc::Rc;
 
 use crate::conversion::{FromPyObject, IntoPyObject, ToPyObject};
@@ -10,7 +11,7 @@ use crate::ffi;
 use crate::instance;
 use crate::object::PyObject;
 use crate::objectprotocol::ObjectProtocol;
-use crate::python::{IntoPyPointer, Python, ToPyPointer};
+use crate::python::{IntoPyPointer, Python, ToPyPointer, NonNullPyObject};
 use crate::pythonrun;
 use crate::typeob::PyTypeCreate;
 use crate::typeob::{PyTypeInfo, PyTypeObject};
@@ -96,7 +97,7 @@ pub trait AsPyRef<T>: Sized {
 
 /// Safe wrapper around unsafe `*mut ffi::PyObject` pointer with specified type information.
 #[derive(Debug)]
-pub struct Py<T>(pub *mut ffi::PyObject, std::marker::PhantomData<T>);
+pub struct Py<T>(NonNullPyObject, 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> {}
@@ -112,18 +113,17 @@ impl<T> Py<T> {
             !ptr.is_null() && ffi::Py_REFCNT(ptr) > 0,
             format!("REFCNT: {:?} - {:?}", ptr, ffi::Py_REFCNT(ptr))
         );
-        Py(ptr, std::marker::PhantomData)
+        Py(NonNull::new_unchecked(ptr), std::marker::PhantomData)
     }
 
     /// Creates a `Py<T>` instance for the given FFI pointer.
     /// Panics if the pointer is `null`.
     /// Undefined behavior if the pointer is invalid.
     #[inline]
     pub unsafe fn from_owned_ptr_or_panic(ptr: *mut ffi::PyObject) -> Py<T> {
-        if ptr.is_null() {
-            crate::err::panic_after_error();
-        } else {
-            Py::from_owned_ptr(ptr)
+        match NonNull::new(ptr) {
+            Some(nonnull_ptr) => { Py(nonnull_ptr, std::marker::PhantomData) },
+            None => { crate::err::panic_after_error(); }
         }
     }
 
@@ -132,10 +132,9 @@ impl<T> Py<T> {
     /// Returns `Err(PyErr)` if the pointer is `null`.
     /// Unsafe because the pointer might be invalid.
     pub unsafe fn from_owned_ptr_or_err(py: Python, ptr: *mut ffi::PyObject) -> PyResult<Py<T>> {
-        if ptr.is_null() {
-            Err(PyErr::fetch(py))
-        } else {
-            Ok(Py::from_owned_ptr(ptr))
+        match NonNull::new(ptr) {
+            Some(nonnull_ptr) => { Ok(Py(nonnull_ptr, std::marker::PhantomData)) },
+            None => { Err(PyErr::fetch(py)) }
         }
     }
 
@@ -149,19 +148,19 @@ impl<T> Py<T> {
             format!("REFCNT: {:?} - {:?}", ptr, ffi::Py_REFCNT(ptr))
         );
         ffi::Py_INCREF(ptr);
-        Py::from_owned_ptr(ptr)
+        Py(NonNull::new_unchecked(ptr), std::marker::PhantomData)
     }
 
     /// Gets the reference count of the ffi::PyObject pointer.
     #[inline]
     pub fn get_refcnt(&self) -> isize {
-        unsafe { ffi::Py_REFCNT(self.0) }
+        unsafe { ffi::Py_REFCNT(self.0.as_ptr()) }
     }
 
     /// Clone self, Calls Py_INCREF() on the ptr.
     #[inline]
     pub fn clone_ref(&self, _py: Python) -> Py<T> {
-        unsafe { Py::from_borrowed_ptr(self.0) }
+        unsafe { Py::from_borrowed_ptr(self.0.as_ptr()) }
     }
 }
 
@@ -241,9 +240,11 @@ impl<T> AsPyRef<T> for Py<T>
 where
     T: PyTypeInfo,
 {
+    #[inline]
     fn as_ref(&self, py: Python) -> &T {
         self.as_ref_dispatch(py)
     }
+    #[inline]
     fn as_mut(&mut self, py: Python) -> &mut T {
         self.as_mut_dispatch(py)
     }
@@ -269,7 +270,7 @@ impl<T> ToPyPointer for Py<T> {
     /// Gets the underlying FFI pointer, returns a borrowed pointer.
     #[inline]
     fn as_ptr(&self) -> *mut ffi::PyObject {
-        self.0
+        self.0.as_ptr()
     }
 }
 
@@ -278,7 +279,7 @@ impl<T> IntoPyPointer for Py<T> {
     #[inline]
     #[must_use]
     fn into_ptr(self) -> *mut ffi::PyObject {
-        let ptr = self.0;
+        let ptr = self.0.as_ptr();
         std::mem::forget(self);
         ptr
     }

diff --git a/src/object.rs b/src/object.rs
@@ -1,14 +1,15 @@
 // Copyright (c) 2017-present PyO3 Project and Contributors
 
 use std;
+use std::ptr::NonNull;
 
 use crate::conversion::{
     FromPyObject, IntoPyObject, IntoPyTuple, PyTryFrom, ToBorrowedObject, ToPyObject,
 };
 use crate::err::{PyDowncastError, PyErr, PyResult};
 use crate::ffi;
 use crate::instance::{AsPyRef, PyObjectWithToken};
-use crate::python::{IntoPyPointer, Python, ToPyPointer};
+use crate::python::{IntoPyPointer, Python, ToPyPointer, NonNullPyObject};
 use crate::pythonrun;
 use crate::types::{PyDict, PyObjectRef, PyTuple};
 
@@ -20,7 +21,7 @@ use crate::types::{PyDict, PyObjectRef, PyTuple};
 /// Technically, it is a safe wrapper around the unsafe `*mut ffi::PyObject` pointer.
 #[derive(Debug)]
 #[repr(transparent)]
-pub struct PyObject(*mut ffi::PyObject);
+pub struct PyObject(NonNullPyObject);
 
 // `PyObject` is thread-safe, any python related operations require a Python<'p> token.
 unsafe impl Send for PyObject {}
@@ -36,40 +37,37 @@ impl PyObject {
             !ptr.is_null() && ffi::Py_REFCNT(ptr) > 0,
             format!("REFCNT: {:?} - {:?}", ptr, ffi::Py_REFCNT(ptr))
         );
-        PyObject(ptr)
+        PyObject(NonNull::new_unchecked(ptr))
     }
 
     /// Creates a `PyObject` instance for the given FFI pointer.
     /// Panics if the pointer is `null`.
     /// Undefined behavior if the pointer is invalid.
     #[inline]
-    pub unsafe fn from_owned_ptr_or_panic(py: Python, ptr: *mut ffi::PyObject) -> PyObject {
-        if ptr.is_null() {
-            crate::err::panic_after_error();
-        } else {
-            PyObject::from_owned_ptr(py, ptr)
+    pub unsafe fn from_owned_ptr_or_panic(_py: Python, ptr: *mut ffi::PyObject) -> PyObject {
+        match NonNull::new(ptr) {
+            Some(nonnull_ptr) => { PyObject(nonnull_ptr) },
+            None => { crate::err::panic_after_error(); }
         }
     }
 
     /// Construct `PyObject` from the result of a Python FFI call that
     /// returns a new reference (owned pointer).
     /// Returns `Err(PyErr)` if the pointer is `null`.
     pub unsafe fn from_owned_ptr_or_err(py: Python, ptr: *mut ffi::PyObject) -> PyResult<PyObject> {
-        if ptr.is_null() {
-            Err(PyErr::fetch(py))
-        } else {
-            Ok(PyObject::from_owned_ptr(py, ptr))
+        match NonNull::new(ptr) {
+            Some(nonnull_ptr) => { Ok(PyObject(nonnull_ptr)) },
+            None => { Err(PyErr::fetch(py)) }
         }
     }
 
     /// Construct `PyObject` from the result of a Python FFI call that
     /// returns a new reference (owned pointer).
     /// Returns `None` if the pointer is `null`.
-    pub unsafe fn from_owned_ptr_or_opt(py: Python, ptr: *mut ffi::PyObject) -> Option<PyObject> {
-        if ptr.is_null() {
-            None
-        } else {
-            Some(PyObject::from_owned_ptr(py, ptr))
+    pub unsafe fn from_owned_ptr_or_opt(_py: Python, ptr: *mut ffi::PyObject) -> Option<PyObject> {
+        match NonNull::new(ptr) {
+            Some(nonnull_ptr) => { Some(PyObject(nonnull_ptr)) },
+            None => { None }
         }
     }
 
@@ -83,7 +81,7 @@ impl PyObject {
             format!("REFCNT: {:?} - {:?}", ptr, ffi::Py_REFCNT(ptr))
         );
         ffi::Py_INCREF(ptr);
-        PyObject(ptr)
+        PyObject(NonNull::new_unchecked(ptr))
     }
 
     /// Creates a `PyObject` instance for the given Python FFI pointer.
@@ -116,7 +114,7 @@ impl PyObject {
 
     /// Gets the reference count of the ffi::PyObject pointer.
     pub fn get_refcnt(&self) -> isize {
-        unsafe { ffi::Py_REFCNT(self.0) }
+        unsafe { ffi::Py_REFCNT(self.0.as_ptr()) }
     }
 
     /// Clone self, Calls Py_INCREF() on the ptr.
@@ -266,15 +264,15 @@ impl ToPyPointer for PyObject {
     /// Gets the underlying FFI pointer, returns a borrowed pointer.
     #[inline]
     fn as_ptr(&self) -> *mut ffi::PyObject {
-        self.0
+        self.0.as_ptr()
     }
 }
 
 impl<'a> ToPyPointer for &'a PyObject {
     /// Gets the underlying FFI pointer, returns a borrowed pointer.
     #[inline]
     fn as_ptr(&self) -> *mut ffi::PyObject {
-        self.0
+        self.0.as_ptr()
     }
 }
 
@@ -283,7 +281,7 @@ impl IntoPyPointer for PyObject {
     #[inline]
     #[must_use]
     fn into_ptr(self) -> *mut ffi::PyObject {
-        let ptr = self.0;
+        let ptr = self.0.as_ptr();
         std::mem::forget(self); // Avoid Drop
         ptr
     }

diff --git a/src/python.rs b/src/python.rs
@@ -11,10 +11,13 @@ use crate::pythonrun::{self, GILGuard};
 use crate::typeob::PyTypeCreate;
 use crate::typeob::{PyTypeInfo, PyTypeObject};
 use crate::types::{PyDict, PyModule, PyObjectRef, PyType};
-use std;
 use std::ffi::CString;
 use std::marker::PhantomData;
 use std::os::raw::c_int;
+use std::ptr::NonNull;
+use std;
+
+pub type NonNullPyObject = NonNull<ffi::PyObject>;
 
 /// Marker type that indicates that the GIL is currently held.
 ///

diff --git a/src/pythonrun.rs b/src/pythonrun.rs
@@ -1,6 +1,6 @@
 // Copyright (c) 2017-present PyO3 Project and Contributors
 use crate::ffi;
-use crate::python::Python;
+use crate::python::{Python, NonNullPyObject};
 use crate::types::PyObjectRef;
 use spin;
 use std::{any, marker, rc, sync};
@@ -230,12 +230,12 @@ pub unsafe fn register_any<'p, T: 'static>(obj: T) -> &'p T {
         .unwrap()
 }
 
-pub unsafe fn register_pointer(obj: *mut ffi::PyObject) {
+pub unsafe fn register_pointer(obj: NonNullPyObject) {
     let pool: &'static mut ReleasePool = &mut *POOL;
 
     let mut v = pool.p.lock();
     let pool: &'static mut Vec<*mut ffi::PyObject> = &mut *(*v);
-    pool.push(obj);
+    pool.push(obj.as_ptr());
 }
 
 pub unsafe fn register_owned(_py: Python, obj: *mut ffi::PyObject) -> &PyObjectRef {

diff --git a/src/typeob.rs b/src/typeob.rs
@@ -163,6 +163,7 @@ impl PyRawObject {
 }
 
 impl<T: PyTypeInfo> AsRef<T> for PyRawObject {
+    #[inline]
     fn as_ref(&self) -> &T {
         // TODO: check is object initialized
         unsafe {

diff --git a/src/types/mod.rs b/src/types/mod.rs
@@ -60,6 +60,7 @@ macro_rules! pyobject_native_type_named (
         impl<$($type_param,)*> $crate::PyNativeType for $name {}
 
         impl<$($type_param,)*> ::std::convert::AsRef<$crate::types::PyObjectRef> for $name {
+            #[inline]
             fn as_ref(&self) -> &$crate::types::PyObjectRef {
                 unsafe{&*(self as *const $name as *const $crate::types::PyObjectRef)}
             }