From 50c612960725b51c4037db2d45f06e9e2d0c1f8b Mon Sep 17 00:00:00 2001 From: ijl Date: Thu, 8 Nov 2018 15:09:52 +0000 Subject: [PATCH] NonNull pointer for Py, PyObject --- src/instance.rs | 33 +++++++++++++++++---------------- src/object.rs | 42 ++++++++++++++++++++---------------------- src/python.rs | 5 ++++- src/pythonrun.rs | 6 +++--- src/typeob.rs | 1 + src/types/mod.rs | 1 + 6 files changed, 46 insertions(+), 42 deletions(-) diff --git a/src/instance.rs b/src/instance.rs index b0a0cc357c5..de810da7efe 100644 --- 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: Sized { /// Safe wrapper around unsafe `*mut ffi::PyObject` pointer with specified type information. #[derive(Debug)] -pub struct Py(pub *mut ffi::PyObject, std::marker::PhantomData); +pub struct Py(NonNullPyObject, std::marker::PhantomData); // `Py` is thread-safe, because any python related operations require a Python<'p> token. unsafe impl Send for Py {} @@ -112,7 +113,7 @@ impl Py { !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` instance for the given FFI pointer. @@ -120,10 +121,9 @@ impl Py { /// Undefined behavior if the pointer is invalid. #[inline] pub unsafe fn from_owned_ptr_or_panic(ptr: *mut ffi::PyObject) -> Py { - 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 Py { /// 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> { - 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 Py { 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 { - unsafe { Py::from_borrowed_ptr(self.0) } + unsafe { Py::from_borrowed_ptr(self.0.as_ptr()) } } } @@ -241,9 +240,11 @@ impl AsPyRef for Py 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 ToPyPointer for Py { /// 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 IntoPyPointer for Py { #[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 index 48866c6ea19..ef9a0229d20 100644 --- a/src/object.rs +++ b/src/object.rs @@ -1,6 +1,7 @@ // Copyright (c) 2017-present PyO3 Project and Contributors use std; +use std::ptr::NonNull; use crate::conversion::{ FromPyObject, IntoPyObject, IntoPyTuple, PyTryFrom, ToBorrowedObject, ToPyObject, @@ -8,7 +9,7 @@ use crate::conversion::{ 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,18 +37,17 @@ 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(); } } } @@ -55,21 +55,19 @@ impl PyObject { /// 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 { - 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 { - 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 { + 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,7 +264,7 @@ 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() } } @@ -274,7 +272,7 @@ 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 index cd316f8812b..6ef751b7c4e 100644 --- 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; /// Marker type that indicates that the GIL is currently held. /// diff --git a/src/pythonrun.rs b/src/pythonrun.rs index a37aefa39f4..7a0dbbc49fd 100644 --- 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 index 9cffc33aa34..266116d3fa0 100644 --- a/src/typeob.rs +++ b/src/typeob.rs @@ -163,6 +163,7 @@ impl PyRawObject { } impl AsRef 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 index 6d6b20e3286..a92db71b936 100644 --- 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)} }