From e0219b44e58d1f22ab011c66a164e9e3a8bef742 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Tue, 9 Nov 2021 20:21:40 +0100 Subject: [PATCH 01/35] Add PyCapsule API --- src/lib.rs | 1 + src/pycapsule.rs | 232 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 233 insertions(+) create mode 100644 src/pycapsule.rs diff --git a/src/lib.rs b/src/lib.rs index 2d0ca39ba70..494d0143994 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -334,6 +334,7 @@ pub mod marshal; pub mod once_cell; pub mod panic; pub mod prelude; +pub mod pycapsule; pub mod pycell; pub mod pyclass; pub mod pyclass_init; diff --git a/src/pycapsule.rs b/src/pycapsule.rs new file mode 100644 index 00000000000..fa983c5cb9e --- /dev/null +++ b/src/pycapsule.rs @@ -0,0 +1,232 @@ +use crate::Python; +use crate::{ffi, AsPyPointer, PyAny}; +use crate::{pyobject_native_type_core, PyErr, PyResult}; +use std::ffi::{c_void, CStr}; +use std::os::raw::c_int; + +/// TODO: docs +/// +#[repr(transparent)] +pub struct PyCapsule(PyAny); + +pyobject_native_type_core!(PyCapsule, ffi::PyCapsule_Type, #checkfunction=ffi::PyCapsule_CheckExact); + +impl PyCapsule { + /// TODO: docs + pub fn new<'py, T>( + py: Python<'py>, + value: T, + name: &CStr, + destructor: Option, + ) -> PyResult<&'py Self> { + let val = Box::new(value); + + let cap_ptr = unsafe { + ffi::PyCapsule_New(Box::into_raw(val) as *mut c_void, name.as_ptr(), destructor) + }; + if cap_ptr.is_null() { + Err(PyErr::fetch(py)) + } else { + Ok(unsafe { py.from_owned_ptr::(cap_ptr) }) + } + } + + /// TODO: docs + pub fn import<'py, T>(py: Python<'py>, name: &CStr, no_block: bool) -> PyResult<&'py T> { + let ptr = unsafe { ffi::PyCapsule_Import(name.as_ptr(), no_block as c_int) }; + if ptr.is_null() { + Err(PyErr::fetch(py)) + } else { + Ok(unsafe { &*(ptr as *const T) }) + } + } + + /// TODO: docs + pub fn set_context(&self, py: Python, context: T) -> PyResult<()> { + let ctx = Box::new(context); + let result = + unsafe { ffi::PyCapsule_SetContext(self.as_ptr(), Box::into_raw(ctx) as _) as u8 }; + if result != 0 { + Err(PyErr::fetch(py)) + } else { + Ok(()) + } + } + + /// TODO: docs + pub fn get_context(&self, py: Python) -> PyResult> { + let ctx = unsafe { ffi::PyCapsule_GetContext(self.as_ptr()) }; + if ctx.is_null() { + if self.is_valid() & PyErr::occurred(py) { + Err(PyErr::fetch(py)) + } else { + Ok(None) + } + } else { + Ok(Some(unsafe { &*(ctx as *const T) })) + } + } + + /// TODO: docs + pub fn reference(&self) -> &T { + unsafe { &*(self.get_pointer() as *const T) } + } + + /// TODO: docs + pub fn get_pointer(&self) -> *mut c_void { + unsafe { ffi::PyCapsule_GetPointer(self.0.as_ptr(), self.name().as_ptr()) } + } + + /// TODO: docs + pub fn is_valid(&self) -> bool { + let r = unsafe { ffi::PyCapsule_IsValid(self.as_ptr(), self.name().as_ptr()) } as u8; + r != 0 + } + + /// TODO: docs + pub fn get_deconstructor(&self, py: Python) -> PyResult> { + match unsafe { ffi::PyCapsule_GetDestructor(self.as_ptr()) } { + Some(deconstructor) => Ok(Some(deconstructor)), + None => { + // A None can mean an error was raised, or there is no deconstructor + // https://docs.python.org/3/c-api/capsule.html#c.PyCapsule_GetDestructor + if self.is_valid() { + Ok(None) + } else { + Err(PyErr::fetch(py)) + } + } + } + } + + /// TODO: docs + pub fn name(&self) -> &CStr { + unsafe { + let ptr = ffi::PyCapsule_GetName(self.as_ptr()); + CStr::from_ptr(ptr) + } + } +} + +#[cfg(test)] +mod tests { + use crate::prelude::PyModule; + use crate::{ffi, pycapsule::PyCapsule, PyResult, Python}; + use std::ffi::{c_void, CString}; + use std::sync::mpsc::{channel, Sender}; + + #[test] + fn test_pycapsule_struct() -> PyResult<()> { + #[repr(C)] + struct Foo { + pub val: u32, + } + + impl Foo { + fn get_val(&self) -> u32 { + self.val + } + } + + Python::with_gil(|py| -> PyResult<()> { + let foo = Foo { val: 123 }; + let name = CString::new("foo").unwrap(); + + let cap = PyCapsule::new(py, foo, &name, None)?; + assert!(cap.is_valid()); + + let foo_capi = cap.reference::(); + assert_eq!(foo_capi.val, 123); + assert_eq!(foo_capi.get_val(), 123); + assert_eq!(cap.name(), name.as_ref()); + Ok(()) + }) + } + + #[test] + fn test_pycapsule_func() -> PyResult<()> { + extern "C" fn foo(x: u32) -> u32 { + x + } + + Python::with_gil(|py| { + let name = CString::new("foo").unwrap(); + + let cap = PyCapsule::new(py, foo as *const c_void, &name, None)?; + let f = cap.reference:: u32>(); + assert_eq!(f(123), 123); + Ok(()) + }) + } + + #[test] + fn test_pycapsule_context() -> PyResult<()> { + Python::with_gil(|py| { + let name = CString::new("foo").unwrap(); + let cap = PyCapsule::new(py, (), &name, None)?; + + let c = cap.get_context::<()>(py)?; + assert!(c.is_none()); + + cap.set_context(py, 123)?; + + let ctx: Option<&u32> = cap.get_context(py)?; + assert_eq!(ctx, Some(&123)); + Ok(()) + }) + } + + #[test] + fn test_pycapsule_import() -> PyResult<()> { + #[repr(C)] + struct Foo { + pub val: u32, + } + + Python::with_gil(|py| -> PyResult<()> { + let foo = Foo { val: 123 }; + let name = CString::new("builtins.capsule").unwrap(); + + let capsule = PyCapsule::new(py, foo, &name, None)?; + + let module = PyModule::import(py, "builtins")?; + module.add("capsule", capsule)?; + + let path = CString::new("builtins.capsule").unwrap(); + let cap: &Foo = PyCapsule::import(py, path.as_ref(), false)?; + assert_eq!(cap.val, 123); + Ok(()) + }) + } + + #[test] + fn test_pycapsule_destructor() { + #[repr(C)] + struct Foo { + called: Sender, + } + + let (tx, rx) = channel(); + + // Setup destructor, call sender to notify of being called + unsafe extern "C" fn destructor(ptr: *mut ffi::PyObject) { + Python::with_gil(|py| { + let cap = py.from_borrowed_ptr::(ptr); + let foo = cap.reference::(); + foo.called.send(true).unwrap(); + }) + } + + // Create a capsule and allow it to be freed. + let r = Python::with_gil(|py| -> PyResult<()> { + let foo = Foo { called: tx }; + let name = CString::new("builtins.capsule").unwrap(); + let _capsule = PyCapsule::new(py, foo, &name, Some(destructor))?; + Ok(()) + }); + assert!(r.is_ok()); + + // Indeed it was + assert_eq!(rx.recv(), Ok(true)); + } +} From ed21e8cb742c80cbe89ec01b81d55cda0b127e7c Mon Sep 17 00:00:00 2001 From: milesgranger Date: Fri, 12 Nov 2021 15:49:19 +0100 Subject: [PATCH 02/35] Docs and small cleanup --- src/pycapsule.rs | 78 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 61 insertions(+), 17 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index fa983c5cb9e..667501e9f70 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -1,18 +1,53 @@ +// Copyright (c) 2017-present PyO3 Project and Contributors use crate::Python; use crate::{ffi, AsPyPointer, PyAny}; use crate::{pyobject_native_type_core, PyErr, PyResult}; use std::ffi::{c_void, CStr}; use std::os::raw::c_int; -/// TODO: docs -/// +/// Represents a Python Capsule +/// As described in [Capsules](https://docs.python.org/3/c-api/capsule.html#capsules) +/// > This subtype of PyObject represents an opaque value, useful for C extension +/// > modules who need to pass an opaque value (as a void* pointer) through Python +/// > code to other C code. It is often used to make a C function pointer defined +/// > in one module available to other modules, so the regular import mechanism can +/// > be used to access C APIs defined in dynamically loaded modules. +/// +/// +/// # Example +/// ``` +/// use std::ffi::CString; +/// use pyo3::{prelude::*, pycapsule::PyCapsule}; +/// +/// #[repr(C)] +/// struct Foo { +/// pub val: u32, +/// } +/// +/// let r = Python::with_gil(|py| -> PyResult<()> { +/// let foo = Foo { val: 123 }; +/// let name = CString::new("builtins.capsule").unwrap(); +/// +/// let capsule = PyCapsule::new(py, foo, name.as_ref(), None)?; +/// +/// let module = PyModule::import(py, "builtins")?; +/// module.add("capsule", capsule)?; +/// +/// let cap: &Foo = PyCapsule::import(py, name.as_ref(), false)?; +/// assert_eq!(cap.val, 123); +/// Ok(()) +/// }); +/// assert!(r.is_ok()); +/// ``` #[repr(transparent)] pub struct PyCapsule(PyAny); pyobject_native_type_core!(PyCapsule, ffi::PyCapsule_Type, #checkfunction=ffi::PyCapsule_CheckExact); impl PyCapsule { - /// TODO: docs + /// Constructs a new capsule of whose contents are `T` associated with `name`. + /// Can optionally provide a deconstructor for when `PyCapsule` is destroyed + /// it will be passed the capsule. pub fn new<'py, T>( py: Python<'py>, value: T, @@ -31,7 +66,14 @@ impl PyCapsule { } } - /// TODO: docs + /// Import an existing capsule. + /// + /// The `name` should match the path to module attribute exactly in the form + /// of `module.attribute`, which should be the same as the name within the + /// capsule. `no_block` indicates to use + /// [PyImport_ImportModuleNoBlock()](https://docs.python.org/3/c-api/import.html#c.PyImport_ImportModuleNoBlock) + /// or [PyImport_ImportModule()](https://docs.python.org/3/c-api/import.html#c.PyImport_ImportModule) + /// when accessing the capsule. pub fn import<'py, T>(py: Python<'py>, name: &CStr, no_block: bool) -> PyResult<&'py T> { let ptr = unsafe { ffi::PyCapsule_Import(name.as_ptr(), no_block as c_int) }; if ptr.is_null() { @@ -41,7 +83,7 @@ impl PyCapsule { } } - /// TODO: docs + /// Set a context pointer in the capsule to `T` pub fn set_context(&self, py: Python, context: T) -> PyResult<()> { let ctx = Box::new(context); let result = @@ -53,7 +95,7 @@ impl PyCapsule { } } - /// TODO: docs + /// Get a reference to the context `T` in the capsule, if any. pub fn get_context(&self, py: Python) -> PyResult> { let ctx = unsafe { ffi::PyCapsule_GetContext(self.as_ptr()) }; if ctx.is_null() { @@ -67,23 +109,26 @@ impl PyCapsule { } } - /// TODO: docs - pub fn reference(&self) -> &T { - unsafe { &*(self.get_pointer() as *const T) } + /// Obtain a reference to the value `T` of this capsule. + /// + /// # Safety + /// This is unsafe because there is no guarantee the pointer is `T` + pub unsafe fn reference(&self) -> &T { + &*(self.get_pointer() as *const T) } - /// TODO: docs + /// Get the raw `c_void` pointer to the value in this capsule. pub fn get_pointer(&self) -> *mut c_void { unsafe { ffi::PyCapsule_GetPointer(self.0.as_ptr(), self.name().as_ptr()) } } - /// TODO: docs + /// Check if this is a valid capsule. pub fn is_valid(&self) -> bool { let r = unsafe { ffi::PyCapsule_IsValid(self.as_ptr(), self.name().as_ptr()) } as u8; r != 0 } - /// TODO: docs + /// Get the capsule deconstructor, if any. pub fn get_deconstructor(&self, py: Python) -> PyResult> { match unsafe { ffi::PyCapsule_GetDestructor(self.as_ptr()) } { Some(deconstructor) => Ok(Some(deconstructor)), @@ -99,7 +144,7 @@ impl PyCapsule { } } - /// TODO: docs + /// Retrieve the name of this capsule. pub fn name(&self) -> &CStr { unsafe { let ptr = ffi::PyCapsule_GetName(self.as_ptr()); @@ -135,7 +180,7 @@ mod tests { let cap = PyCapsule::new(py, foo, &name, None)?; assert!(cap.is_valid()); - let foo_capi = cap.reference::(); + let foo_capi = unsafe { cap.reference::() }; assert_eq!(foo_capi.val, 123); assert_eq!(foo_capi.get_val(), 123); assert_eq!(cap.name(), name.as_ref()); @@ -153,7 +198,7 @@ mod tests { let name = CString::new("foo").unwrap(); let cap = PyCapsule::new(py, foo as *const c_void, &name, None)?; - let f = cap.reference:: u32>(); + let f = unsafe { cap.reference:: u32>() }; assert_eq!(f(123), 123); Ok(()) }) @@ -192,8 +237,7 @@ mod tests { let module = PyModule::import(py, "builtins")?; module.add("capsule", capsule)?; - let path = CString::new("builtins.capsule").unwrap(); - let cap: &Foo = PyCapsule::import(py, path.as_ref(), false)?; + let cap: &Foo = PyCapsule::import(py, name.as_ref(), false)?; assert_eq!(cap.val, 123); Ok(()) }) From a7ca02d80c2a2ca77decc1af76bff27c10de9cd5 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sat, 13 Nov 2021 13:46:20 +0100 Subject: [PATCH 03/35] Add T: 'static and vec tests for context and storage --- src/pycapsule.rs | 41 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index 667501e9f70..efdfe7a3f8e 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -48,7 +48,7 @@ impl PyCapsule { /// Constructs a new capsule of whose contents are `T` associated with `name`. /// Can optionally provide a deconstructor for when `PyCapsule` is destroyed /// it will be passed the capsule. - pub fn new<'py, T>( + pub fn new<'py, T: 'static>( py: Python<'py>, value: T, name: &CStr, @@ -84,7 +84,7 @@ impl PyCapsule { } /// Set a context pointer in the capsule to `T` - pub fn set_context(&self, py: Python, context: T) -> PyResult<()> { + pub fn set_context<'py, T: 'static>(&self, py: Python<'py>, context: T) -> PyResult<()> { let ctx = Box::new(context); let result = unsafe { ffi::PyCapsule_SetContext(self.as_ptr(), Box::into_raw(ctx) as _) as u8 }; @@ -156,7 +156,7 @@ impl PyCapsule { #[cfg(test)] mod tests { use crate::prelude::PyModule; - use crate::{ffi, pycapsule::PyCapsule, PyResult, Python}; + use crate::{ffi, pycapsule::PyCapsule, PyResult, Python, Py}; use std::ffi::{c_void, CString}; use std::sync::mpsc::{channel, Sender}; @@ -273,4 +273,39 @@ mod tests { // Indeed it was assert_eq!(rx.recv(), Ok(true)); } + + #[test] + fn test_vec_storage() { + let cap: Py = Python::with_gil(|py| { + let name = CString::new("foo").unwrap(); + + let stuff: Vec = vec![1, 2, 3, 4]; + let cap = PyCapsule::new(py, stuff, &name, None).unwrap(); + + cap.into() + }); + + Python::with_gil(|py| { + let ctx: &Vec = unsafe { cap.as_ref(py).reference() }; + assert_eq!(ctx, &[1, 2, 3, 4]); + }) + } + + #[test] + fn test_vec_context() { + let cap: Py = Python::with_gil(|py| { + let name = CString::new("foo").unwrap(); + let cap = PyCapsule::new(py, (), &name, None).unwrap(); + + let ctx: Vec = vec![1, 2, 3, 4]; + cap.set_context(py, ctx).unwrap(); + + cap.into() + }); + + Python::with_gil(|py| { + let ctx: Option<&Vec> = cap.as_ref(py).get_context(py).unwrap(); + assert_eq!(ctx, Some(&vec![1_u8, 2, 3, 4])); + }) + } } From 295b6115fe1cee3e66bdf753e8c0c107e4d6a6bf Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sat, 13 Nov 2021 14:02:47 +0100 Subject: [PATCH 04/35] Update func test to use Py, still pass --- src/pycapsule.rs | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index efdfe7a3f8e..33175e9ae83 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -156,7 +156,7 @@ impl PyCapsule { #[cfg(test)] mod tests { use crate::prelude::PyModule; - use crate::{ffi, pycapsule::PyCapsule, PyResult, Python, Py}; + use crate::{ffi, pycapsule::PyCapsule, Py, PyResult, Python}; use std::ffi::{c_void, CString}; use std::sync::mpsc::{channel, Sender}; @@ -189,19 +189,21 @@ mod tests { } #[test] - fn test_pycapsule_func() -> PyResult<()> { - extern "C" fn foo(x: u32) -> u32 { - x - } + fn test_pycapsule_func() { + let cap: Py = Python::with_gil(|py| { + extern "C" fn foo(x: u32) -> u32 { + x + } - Python::with_gil(|py| { let name = CString::new("foo").unwrap(); + let cap = PyCapsule::new(py, foo as *const c_void, &name, None).unwrap(); + cap.into() + }); - let cap = PyCapsule::new(py, foo as *const c_void, &name, None)?; - let f = unsafe { cap.reference:: u32>() }; + Python::with_gil(|py| { + let f = unsafe { cap.as_ref(py).reference:: u32>() }; assert_eq!(f(123), 123); - Ok(()) - }) + }); } #[test] @@ -278,13 +280,13 @@ mod tests { fn test_vec_storage() { let cap: Py = Python::with_gil(|py| { let name = CString::new("foo").unwrap(); - + let stuff: Vec = vec![1, 2, 3, 4]; let cap = PyCapsule::new(py, stuff, &name, None).unwrap(); - + cap.into() }); - + Python::with_gil(|py| { let ctx: &Vec = unsafe { cap.as_ref(py).reference() }; assert_eq!(ctx, &[1, 2, 3, 4]); @@ -296,13 +298,13 @@ mod tests { let cap: Py = Python::with_gil(|py| { let name = CString::new("foo").unwrap(); let cap = PyCapsule::new(py, (), &name, None).unwrap(); - + let ctx: Vec = vec![1, 2, 3, 4]; cap.set_context(py, ctx).unwrap(); - + cap.into() }); - + Python::with_gil(|py| { let ctx: Option<&Vec> = cap.as_ref(py).get_context(py).unwrap(); assert_eq!(ctx, Some(&vec![1_u8, 2, 3, 4])); From 12edeffb1dc63b82492f8d565dcfc88934e1add6 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Thu, 18 Nov 2021 08:32:40 +0100 Subject: [PATCH 05/35] Make PyCapsule::import unsafe --- src/pycapsule.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index 33175e9ae83..72796e9849e 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -74,12 +74,15 @@ impl PyCapsule { /// [PyImport_ImportModuleNoBlock()](https://docs.python.org/3/c-api/import.html#c.PyImport_ImportModuleNoBlock) /// or [PyImport_ImportModule()](https://docs.python.org/3/c-api/import.html#c.PyImport_ImportModule) /// when accessing the capsule. - pub fn import<'py, T>(py: Python<'py>, name: &CStr, no_block: bool) -> PyResult<&'py T> { - let ptr = unsafe { ffi::PyCapsule_Import(name.as_ptr(), no_block as c_int) }; + /// + /// ## Safety + /// This is unsafe, as there is no gurantee when casting `*mut void` into `T`. + pub unsafe fn import<'py, T>(py: Python<'py>, name: &CStr, no_block: bool) -> PyResult<&'py T> { + let ptr = ffi::PyCapsule_Import(name.as_ptr(), no_block as c_int); if ptr.is_null() { Err(PyErr::fetch(py)) } else { - Ok(unsafe { &*(ptr as *const T) }) + Ok(&*(ptr as *const T)) } } @@ -239,7 +242,7 @@ mod tests { let module = PyModule::import(py, "builtins")?; module.add("capsule", capsule)?; - let cap: &Foo = PyCapsule::import(py, name.as_ref(), false)?; + let cap: &Foo = unsafe { PyCapsule::import(py, name.as_ref(), false)? }; assert_eq!(cap.val, 123); Ok(()) }) From b16ec0b6f105b17181249e403026acbeb6a1fc15 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Thu, 18 Nov 2021 08:36:15 +0100 Subject: [PATCH 06/35] Make PyCapsule::get_context unsafe --- src/pycapsule.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index 72796e9849e..e675be554fc 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -76,7 +76,7 @@ impl PyCapsule { /// when accessing the capsule. /// /// ## Safety - /// This is unsafe, as there is no gurantee when casting `*mut void` into `T`. + /// This is unsafe, as there is no guarantee when casting `*mut void` into `T`. pub unsafe fn import<'py, T>(py: Python<'py>, name: &CStr, no_block: bool) -> PyResult<&'py T> { let ptr = ffi::PyCapsule_Import(name.as_ptr(), no_block as c_int); if ptr.is_null() { @@ -99,8 +99,12 @@ impl PyCapsule { } /// Get a reference to the context `T` in the capsule, if any. - pub fn get_context(&self, py: Python) -> PyResult> { - let ctx = unsafe { ffi::PyCapsule_GetContext(self.as_ptr()) }; + /// + /// ## Safety + /// + /// This is unsafe, as there is no guarantee when casting `*mut void` into `T`. + pub unsafe fn get_context(&self, py: Python) -> PyResult> { + let ctx = ffi::PyCapsule_GetContext(self.as_ptr()); if ctx.is_null() { if self.is_valid() & PyErr::occurred(py) { Err(PyErr::fetch(py)) @@ -108,7 +112,7 @@ impl PyCapsule { Ok(None) } } else { - Ok(Some(unsafe { &*(ctx as *const T) })) + Ok(Some(&*(ctx as *const T))) } } @@ -215,12 +219,12 @@ mod tests { let name = CString::new("foo").unwrap(); let cap = PyCapsule::new(py, (), &name, None)?; - let c = cap.get_context::<()>(py)?; + let c = unsafe { cap.get_context::<()>(py)? }; assert!(c.is_none()); cap.set_context(py, 123)?; - let ctx: Option<&u32> = cap.get_context(py)?; + let ctx: Option<&u32> = unsafe { cap.get_context(py)? }; assert_eq!(ctx, Some(&123)); Ok(()) }) @@ -309,7 +313,7 @@ mod tests { }); Python::with_gil(|py| { - let ctx: Option<&Vec> = cap.as_ref(py).get_context(py).unwrap(); + let ctx: Option<&Vec> = unsafe { cap.as_ref(py).get_context(py).unwrap() }; assert_eq!(ctx, Some(&vec![1_u8, 2, 3, 4])); }) } From 83f74bb8cc51f41a652de04a98bfef51e498d21b Mon Sep 17 00:00:00 2001 From: milesgranger Date: Thu, 18 Nov 2021 08:38:48 +0100 Subject: [PATCH 07/35] Fix typo in destructor --- src/pycapsule.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index e675be554fc..da0151c3a80 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -46,7 +46,7 @@ pyobject_native_type_core!(PyCapsule, ffi::PyCapsule_Type, #checkfunction=ffi::P impl PyCapsule { /// Constructs a new capsule of whose contents are `T` associated with `name`. - /// Can optionally provide a deconstructor for when `PyCapsule` is destroyed + /// Can optionally provide a destructor for when `PyCapsule` is destroyed /// it will be passed the capsule. pub fn new<'py, T: 'static>( py: Python<'py>, @@ -135,12 +135,12 @@ impl PyCapsule { r != 0 } - /// Get the capsule deconstructor, if any. - pub fn get_deconstructor(&self, py: Python) -> PyResult> { + /// Get the capsule destructor, if any. + pub fn get_destructor(&self, py: Python) -> PyResult> { match unsafe { ffi::PyCapsule_GetDestructor(self.as_ptr()) } { - Some(deconstructor) => Ok(Some(deconstructor)), + Some(destructor) => Ok(Some(destructor)), None => { - // A None can mean an error was raised, or there is no deconstructor + // A None can mean an error was raised, or there is no destructor // https://docs.python.org/3/c-api/capsule.html#c.PyCapsule_GetDestructor if self.is_valid() { Ok(None) From 74e570f4f58a75b4ce33f43da289e02669fc8303 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Thu, 18 Nov 2021 08:49:18 +0100 Subject: [PATCH 08/35] Refactor PyCapsule::new/new_with_destructor --- src/pycapsule.rs | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index da0151c3a80..a0a787b9aa2 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -46,9 +46,22 @@ pyobject_native_type_core!(PyCapsule, ffi::PyCapsule_Type, #checkfunction=ffi::P impl PyCapsule { /// Constructs a new capsule of whose contents are `T` associated with `name`. - /// Can optionally provide a destructor for when `PyCapsule` is destroyed - /// it will be passed the capsule. - pub fn new<'py, T: 'static>( + pub fn new<'py, T: 'static>(py: Python<'py>, value: T, name: &CStr) -> PyResult<&'py Self> { + Self::new_with_maybe_destructor(py, value, name, None) + } + + /// Constructs a new capsule of whose contents are `T` associated with `name` + /// Provide a destructor for when `PyCapsule` is destroyed it will be passed the capsule. + pub fn new_with_destructor<'py, T: 'static>( + py: Python<'py>, + value: T, + name: &CStr, + destructor: ffi::PyCapsule_Destructor, + ) -> PyResult<&'py Self> { + Self::new_with_maybe_destructor(py, value, name, Some(destructor)) + } + + fn new_with_maybe_destructor<'py, T: 'static>( py: Python<'py>, value: T, name: &CStr, @@ -184,7 +197,7 @@ mod tests { let foo = Foo { val: 123 }; let name = CString::new("foo").unwrap(); - let cap = PyCapsule::new(py, foo, &name, None)?; + let cap = PyCapsule::new(py, foo, &name)?; assert!(cap.is_valid()); let foo_capi = unsafe { cap.reference::() }; @@ -203,7 +216,7 @@ mod tests { } let name = CString::new("foo").unwrap(); - let cap = PyCapsule::new(py, foo as *const c_void, &name, None).unwrap(); + let cap = PyCapsule::new(py, foo as *const c_void, &name).unwrap(); cap.into() }); @@ -217,7 +230,7 @@ mod tests { fn test_pycapsule_context() -> PyResult<()> { Python::with_gil(|py| { let name = CString::new("foo").unwrap(); - let cap = PyCapsule::new(py, (), &name, None)?; + let cap = PyCapsule::new(py, (), &name)?; let c = unsafe { cap.get_context::<()>(py)? }; assert!(c.is_none()); @@ -241,7 +254,7 @@ mod tests { let foo = Foo { val: 123 }; let name = CString::new("builtins.capsule").unwrap(); - let capsule = PyCapsule::new(py, foo, &name, None)?; + let capsule = PyCapsule::new(py, foo, &name)?; let module = PyModule::import(py, "builtins")?; module.add("capsule", capsule)?; @@ -274,7 +287,7 @@ mod tests { let r = Python::with_gil(|py| -> PyResult<()> { let foo = Foo { called: tx }; let name = CString::new("builtins.capsule").unwrap(); - let _capsule = PyCapsule::new(py, foo, &name, Some(destructor))?; + let _capsule = PyCapsule::new_with_destructor(py, foo, &name, destructor)?; Ok(()) }); assert!(r.is_ok()); @@ -289,7 +302,7 @@ mod tests { let name = CString::new("foo").unwrap(); let stuff: Vec = vec![1, 2, 3, 4]; - let cap = PyCapsule::new(py, stuff, &name, None).unwrap(); + let cap = PyCapsule::new(py, stuff, &name).unwrap(); cap.into() }); @@ -304,7 +317,7 @@ mod tests { fn test_vec_context() { let cap: Py = Python::with_gil(|py| { let name = CString::new("foo").unwrap(); - let cap = PyCapsule::new(py, (), &name, None).unwrap(); + let cap = PyCapsule::new(py, (), &name).unwrap(); let ctx: Vec = vec![1, 2, 3, 4]; cap.set_context(py, ctx).unwrap(); From 1318cec455715fa19245ee153080d9976d5e5c24 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Thu, 18 Nov 2021 09:33:29 +0100 Subject: [PATCH 09/35] Refactor destructor API [skip ci] --- src/pycapsule.rs | 65 +++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 34 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index a0a787b9aa2..dc3585a0a9e 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -44,33 +44,44 @@ pub struct PyCapsule(PyAny); pyobject_native_type_core!(PyCapsule, ffi::PyCapsule_Type, #checkfunction=ffi::PyCapsule_CheckExact); +struct CapsuleContents { + value: T, + destructor: D, +} +impl CapsuleContents { + fn new(value: T, destructor: D) -> Self { + Self { value, destructor } + } +} + +unsafe extern "C" fn capsule_destructor(capsule: *mut ffi::PyObject) { + let ptr = ffi::PyCapsule_GetPointer(capsule, ffi::PyCapsule_GetName(capsule)); + let CapsuleContents { value, destructor } = *Box::from_raw(ptr as *mut CapsuleContents); + destructor(value) +} + impl PyCapsule { /// Constructs a new capsule of whose contents are `T` associated with `name`. pub fn new<'py, T: 'static>(py: Python<'py>, value: T, name: &CStr) -> PyResult<&'py Self> { - Self::new_with_maybe_destructor(py, value, name, None) + Self::new_with_destructor(py, value, name, std::mem::drop) } /// Constructs a new capsule of whose contents are `T` associated with `name` /// Provide a destructor for when `PyCapsule` is destroyed it will be passed the capsule. - pub fn new_with_destructor<'py, T: 'static>( - py: Python<'py>, - value: T, - name: &CStr, - destructor: ffi::PyCapsule_Destructor, - ) -> PyResult<&'py Self> { - Self::new_with_maybe_destructor(py, value, name, Some(destructor)) - } - - fn new_with_maybe_destructor<'py, T: 'static>( + pub fn new_with_destructor<'py, T: 'static, F: FnOnce(T)>( py: Python<'py>, value: T, name: &CStr, - destructor: Option, + destructor: F, ) -> PyResult<&'py Self> { - let val = Box::new(value); + let val = Box::new(CapsuleContents::new(value, destructor)); let cap_ptr = unsafe { - ffi::PyCapsule_New(Box::into_raw(val) as *mut c_void, name.as_ptr(), destructor) + ffi::PyCapsule_New( + Box::into_raw(val) as *mut c_void, + name.as_ptr(), + Some(capsule_destructor::), + ) }; if cap_ptr.is_null() { Err(PyErr::fetch(py)) @@ -149,19 +160,9 @@ impl PyCapsule { } /// Get the capsule destructor, if any. - pub fn get_destructor(&self, py: Python) -> PyResult> { - match unsafe { ffi::PyCapsule_GetDestructor(self.as_ptr()) } { - Some(destructor) => Ok(Some(destructor)), - None => { - // A None can mean an error was raised, or there is no destructor - // https://docs.python.org/3/c-api/capsule.html#c.PyCapsule_GetDestructor - if self.is_valid() { - Ok(None) - } else { - Err(PyErr::fetch(py)) - } - } - } + pub fn get_destructor(&self) -> PyResult> { + let val = unsafe { Box::from_raw(self.get_pointer() as *mut &CapsuleContents) }; + Ok(Some(&val.destructor)) } /// Retrieve the name of this capsule. @@ -176,7 +177,7 @@ impl PyCapsule { #[cfg(test)] mod tests { use crate::prelude::PyModule; - use crate::{ffi, pycapsule::PyCapsule, Py, PyResult, Python}; + use crate::{pycapsule::PyCapsule, Py, PyResult, Python}; use std::ffi::{c_void, CString}; use std::sync::mpsc::{channel, Sender}; @@ -275,12 +276,8 @@ mod tests { let (tx, rx) = channel(); // Setup destructor, call sender to notify of being called - unsafe extern "C" fn destructor(ptr: *mut ffi::PyObject) { - Python::with_gil(|py| { - let cap = py.from_borrowed_ptr::(ptr); - let foo = cap.reference::(); - foo.called.send(true).unwrap(); - }) + fn destructor(foo: Foo) { + foo.called.send(true).unwrap(); } // Create a capsule and allow it to be freed. From 2f9a754fe85c757576f2888368cd40974cc5bc2f Mon Sep 17 00:00:00 2001 From: milesgranger Date: Fri, 19 Nov 2021 20:31:32 +0100 Subject: [PATCH 10/35] Fix docs regarding safety --- src/pycapsule.rs | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index dc3585a0a9e..4e5234aeb88 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -44,22 +44,6 @@ pub struct PyCapsule(PyAny); pyobject_native_type_core!(PyCapsule, ffi::PyCapsule_Type, #checkfunction=ffi::PyCapsule_CheckExact); -struct CapsuleContents { - value: T, - destructor: D, -} -impl CapsuleContents { - fn new(value: T, destructor: D) -> Self { - Self { value, destructor } - } -} - -unsafe extern "C" fn capsule_destructor(capsule: *mut ffi::PyObject) { - let ptr = ffi::PyCapsule_GetPointer(capsule, ffi::PyCapsule_GetName(capsule)); - let CapsuleContents { value, destructor } = *Box::from_raw(ptr as *mut CapsuleContents); - destructor(value) -} - impl PyCapsule { /// Constructs a new capsule of whose contents are `T` associated with `name`. pub fn new<'py, T: 'static>(py: Python<'py>, value: T, name: &CStr) -> PyResult<&'py Self> { @@ -99,8 +83,9 @@ impl PyCapsule { /// or [PyImport_ImportModule()](https://docs.python.org/3/c-api/import.html#c.PyImport_ImportModule) /// when accessing the capsule. /// - /// ## Safety - /// This is unsafe, as there is no guarantee when casting `*mut void` into `T`. + /// # Safety + /// + /// It must be known that this capsule's pointer is to an item of type `T`. pub unsafe fn import<'py, T>(py: Python<'py>, name: &CStr, no_block: bool) -> PyResult<&'py T> { let ptr = ffi::PyCapsule_Import(name.as_ptr(), no_block as c_int); if ptr.is_null() { @@ -124,9 +109,9 @@ impl PyCapsule { /// Get a reference to the context `T` in the capsule, if any. /// - /// ## Safety + /// # Safety /// - /// This is unsafe, as there is no guarantee when casting `*mut void` into `T`. + /// It must be known that this capsule's contains a context item of type `T`. pub unsafe fn get_context(&self, py: Python) -> PyResult> { let ctx = ffi::PyCapsule_GetContext(self.as_ptr()); if ctx.is_null() { @@ -143,7 +128,8 @@ impl PyCapsule { /// Obtain a reference to the value `T` of this capsule. /// /// # Safety - /// This is unsafe because there is no guarantee the pointer is `T` + /// + /// It must be known that this capsule's pointer is to an item of type `T`. pub unsafe fn reference(&self) -> &T { &*(self.get_pointer() as *const T) } @@ -174,6 +160,20 @@ impl PyCapsule { } } +// C layout, as PyCapsule::get_reference depends on `T` being first. +#[repr(C)] +struct CapsuleContents { + value: T, + destructor: D, +} + +// Wrapping ffi::PyCapsule_Destructor for a user supplied FnOnce(T) for capsule destructor +unsafe extern "C" fn capsule_destructor(capsule: *mut ffi::PyObject) { + let ptr = ffi::PyCapsule_GetPointer(capsule, ffi::PyCapsule_GetName(capsule)); + let CapsuleContents { value, destructor } = *Box::from_raw(ptr as *mut CapsuleContents); + destructor(value) +} + #[cfg(test)] mod tests { use crate::prelude::PyModule; From 894109fe4a12b05bf706d3b27451ef0427b26697 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Fri, 19 Nov 2021 20:33:39 +0100 Subject: [PATCH 11/35] Remove PyCapsule::get_destructor --- src/pycapsule.rs | 36 +----------------------------------- 1 file changed, 1 insertion(+), 35 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index 4e5234aeb88..ba9b70a235f 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -58,7 +58,7 @@ impl PyCapsule { name: &CStr, destructor: F, ) -> PyResult<&'py Self> { - let val = Box::new(CapsuleContents::new(value, destructor)); + let val = Box::new(CapsuleContents { value, destructor }); let cap_ptr = unsafe { ffi::PyCapsule_New( @@ -145,12 +145,6 @@ impl PyCapsule { r != 0 } - /// Get the capsule destructor, if any. - pub fn get_destructor(&self) -> PyResult> { - let val = unsafe { Box::from_raw(self.get_pointer() as *mut &CapsuleContents) }; - Ok(Some(&val.destructor)) - } - /// Retrieve the name of this capsule. pub fn name(&self) -> &CStr { unsafe { @@ -179,7 +173,6 @@ mod tests { use crate::prelude::PyModule; use crate::{pycapsule::PyCapsule, Py, PyResult, Python}; use std::ffi::{c_void, CString}; - use std::sync::mpsc::{channel, Sender}; #[test] fn test_pycapsule_struct() -> PyResult<()> { @@ -266,33 +259,6 @@ mod tests { }) } - #[test] - fn test_pycapsule_destructor() { - #[repr(C)] - struct Foo { - called: Sender, - } - - let (tx, rx) = channel(); - - // Setup destructor, call sender to notify of being called - fn destructor(foo: Foo) { - foo.called.send(true).unwrap(); - } - - // Create a capsule and allow it to be freed. - let r = Python::with_gil(|py| -> PyResult<()> { - let foo = Foo { called: tx }; - let name = CString::new("builtins.capsule").unwrap(); - let _capsule = PyCapsule::new_with_destructor(py, foo, &name, destructor)?; - Ok(()) - }); - assert!(r.is_ok()); - - // Indeed it was - assert_eq!(rx.recv(), Ok(true)); - } - #[test] fn test_vec_storage() { let cap: Py = Python::with_gil(|py| { From 9540679f1a3d5d68be12b1c3cdac2941007340aa Mon Sep 17 00:00:00 2001 From: milesgranger Date: Fri, 19 Nov 2021 20:34:29 +0100 Subject: [PATCH 12/35] Fix typo bitwise operator [skip ci] --- src/pycapsule.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index ba9b70a235f..a3f4e4bff6d 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -115,7 +115,7 @@ impl PyCapsule { pub unsafe fn get_context(&self, py: Python) -> PyResult> { let ctx = ffi::PyCapsule_GetContext(self.as_ptr()); if ctx.is_null() { - if self.is_valid() & PyErr::occurred(py) { + if self.is_valid() && PyErr::occurred(py) { Err(PyErr::fetch(py)) } else { Ok(None) From 06ae71bb9fca425cb4d4e31d1977310e18fb0cdb Mon Sep 17 00:00:00 2001 From: milesgranger Date: Fri, 19 Nov 2021 20:46:10 +0100 Subject: [PATCH 13/35] Fix clippy --- src/pycapsule.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index a3f4e4bff6d..f1dd98e0444 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -96,7 +96,7 @@ impl PyCapsule { } /// Set a context pointer in the capsule to `T` - pub fn set_context<'py, T: 'static>(&self, py: Python<'py>, context: T) -> PyResult<()> { + pub fn set_context(&self, py: Python, context: T) -> PyResult<()> { let ctx = Box::new(context); let result = unsafe { ffi::PyCapsule_SetContext(self.as_ptr(), Box::into_raw(ctx) as _) as u8 }; From c0c195d0395b425a1e9925b94b3a1fad3601abee Mon Sep 17 00:00:00 2001 From: milesgranger Date: Fri, 19 Nov 2021 20:53:39 +0100 Subject: [PATCH 14/35] Fixup: fix doc test fail --- src/pycapsule.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index f1dd98e0444..2800c6df3d9 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -28,12 +28,12 @@ use std::os::raw::c_int; /// let foo = Foo { val: 123 }; /// let name = CString::new("builtins.capsule").unwrap(); /// -/// let capsule = PyCapsule::new(py, foo, name.as_ref(), None)?; +/// let capsule = PyCapsule::new(py, foo, name.as_ref())?; /// /// let module = PyModule::import(py, "builtins")?; /// module.add("capsule", capsule)?; /// -/// let cap: &Foo = PyCapsule::import(py, name.as_ref(), false)?; +/// let cap: &Foo = unsafe { PyCapsule::import(py, name.as_ref(), false)? }; /// assert_eq!(cap.val, 123); /// Ok(()) /// }); From e0f5a0c638515267420fc3d27b744f2e0fe4e628 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sat, 20 Nov 2021 17:23:00 +0100 Subject: [PATCH 15/35] Add changlog entry and address pr comments --- CHANGELOG.md | 7 +++++++ src/pycapsule.rs | 13 ++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fdb5114c0b..95c978c59f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ PyO3 versions, please see the [migration guide](https://pyo3.rs/latest/migration The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [Unreleased] + +### Added + +- Add `PyCapsule`, exposing the [Capsule API](https://docs.python.org/3/c-api/capsule.html#capsules). [#1980](https://github.com/PyO3/pyo3/pull/1980) + ## [0.15.0] - 2021-11-03 ### Packaging diff --git a/src/pycapsule.rs b/src/pycapsule.rs index 2800c6df3d9..3ac74ea2ede 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -67,11 +67,7 @@ impl PyCapsule { Some(capsule_destructor::), ) }; - if cap_ptr.is_null() { - Err(PyErr::fetch(py)) - } else { - Ok(unsafe { py.from_owned_ptr::(cap_ptr) }) - } + unsafe { py.from_owned_ptr_or_err(cap_ptr) } } /// Import an existing capsule. @@ -253,6 +249,13 @@ mod tests { let module = PyModule::import(py, "builtins")?; module.add("capsule", capsule)?; + // check error when wrong named passed for capsule. + let wrong_name = CString::new("builtins.non_existant").unwrap(); + let result: PyResult<&Foo> = + unsafe { PyCapsule::import(py, wrong_name.as_ref(), false) }; + assert!(result.is_err()); + + // corret name is okay. let cap: &Foo = unsafe { PyCapsule::import(py, name.as_ref(), false)? }; assert_eq!(cap.val, 123); Ok(()) From c9b8337bf3c8db80190179a7c4d101a6772cf505 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sat, 20 Nov 2021 17:36:44 +0100 Subject: [PATCH 16/35] Remove noblock option and add grammatical doc updates --- src/pycapsule.rs | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index 3ac74ea2ede..4b7f71721cc 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -6,7 +6,7 @@ use std::ffi::{c_void, CStr}; use std::os::raw::c_int; /// Represents a Python Capsule -/// As described in [Capsules](https://docs.python.org/3/c-api/capsule.html#capsules) +/// as described in [Capsules](https://docs.python.org/3/c-api/capsule.html#capsules): /// > This subtype of PyObject represents an opaque value, useful for C extension /// > modules who need to pass an opaque value (as a void* pointer) through Python /// > code to other C code. It is often used to make a C function pointer defined @@ -45,13 +45,14 @@ pub struct PyCapsule(PyAny); pyobject_native_type_core!(PyCapsule, ffi::PyCapsule_Type, #checkfunction=ffi::PyCapsule_CheckExact); impl PyCapsule { - /// Constructs a new capsule of whose contents are `T` associated with `name`. + /// Constructs a new capsule whose contents are `value`, associated with `name`. pub fn new<'py, T: 'static>(py: Python<'py>, value: T, name: &CStr) -> PyResult<&'py Self> { Self::new_with_destructor(py, value, name, std::mem::drop) } - /// Constructs a new capsule of whose contents are `T` associated with `name` - /// Provide a destructor for when `PyCapsule` is destroyed it will be passed the capsule. + /// Constructs a new capsule whose contents are `value`, associated with `name`. + /// + /// Also provides a destructor: when the `PyCapsule` is destroyed, it will be passed the original object. pub fn new_with_destructor<'py, T: 'static, F: FnOnce(T)>( py: Python<'py>, value: T, @@ -70,20 +71,16 @@ impl PyCapsule { unsafe { py.from_owned_ptr_or_err(cap_ptr) } } - /// Import an existing capsule. + /// Imports an existing capsule. /// - /// The `name` should match the path to module attribute exactly in the form - /// of `module.attribute`, which should be the same as the name within the - /// capsule. `no_block` indicates to use - /// [PyImport_ImportModuleNoBlock()](https://docs.python.org/3/c-api/import.html#c.PyImport_ImportModuleNoBlock) - /// or [PyImport_ImportModule()](https://docs.python.org/3/c-api/import.html#c.PyImport_ImportModule) - /// when accessing the capsule. + /// The `name` should match the path to the module attribute exactly in the form + /// of `module.attribute`, which should be the same as the name within the capsule. /// /// # Safety /// /// It must be known that this capsule's pointer is to an item of type `T`. - pub unsafe fn import<'py, T>(py: Python<'py>, name: &CStr, no_block: bool) -> PyResult<&'py T> { - let ptr = ffi::PyCapsule_Import(name.as_ptr(), no_block as c_int); + pub unsafe fn import<'py, T>(py: Python<'py>, name: &CStr) -> PyResult<&'py T> { + let ptr = ffi::PyCapsule_Import(name.as_ptr(), false as c_int); if ptr.is_null() { Err(PyErr::fetch(py)) } else { @@ -91,7 +88,7 @@ impl PyCapsule { } } - /// Set a context pointer in the capsule to `T` + /// Sets the context pointer in the capsule to `T`. pub fn set_context(&self, py: Python, context: T) -> PyResult<()> { let ctx = Box::new(context); let result = @@ -103,11 +100,11 @@ impl PyCapsule { } } - /// Get a reference to the context `T` in the capsule, if any. + /// Gets a reference to the context of the capsule, if any. /// /// # Safety /// - /// It must be known that this capsule's contains a context item of type `T`. + /// It must be known that this capsule contains a context of type `T`. pub unsafe fn get_context(&self, py: Python) -> PyResult> { let ctx = ffi::PyCapsule_GetContext(self.as_ptr()); if ctx.is_null() { @@ -121,7 +118,7 @@ impl PyCapsule { } } - /// Obtain a reference to the value `T` of this capsule. + /// Obtains a reference to the value of this capsule. /// /// # Safety /// @@ -130,18 +127,18 @@ impl PyCapsule { &*(self.get_pointer() as *const T) } - /// Get the raw `c_void` pointer to the value in this capsule. + /// Gets the raw `c_void` pointer to the value in this capsule. pub fn get_pointer(&self) -> *mut c_void { unsafe { ffi::PyCapsule_GetPointer(self.0.as_ptr(), self.name().as_ptr()) } } - /// Check if this is a valid capsule. + /// Checks if this is a valid capsule. pub fn is_valid(&self) -> bool { let r = unsafe { ffi::PyCapsule_IsValid(self.as_ptr(), self.name().as_ptr()) } as u8; r != 0 } - /// Retrieve the name of this capsule. + /// Retrieves the name of this capsule. pub fn name(&self) -> &CStr { unsafe { let ptr = ffi::PyCapsule_GetName(self.as_ptr()); @@ -252,11 +249,11 @@ mod tests { // check error when wrong named passed for capsule. let wrong_name = CString::new("builtins.non_existant").unwrap(); let result: PyResult<&Foo> = - unsafe { PyCapsule::import(py, wrong_name.as_ref(), false) }; + unsafe { PyCapsule::import(py, wrong_name.as_ref()) }; assert!(result.is_err()); // corret name is okay. - let cap: &Foo = unsafe { PyCapsule::import(py, name.as_ref(), false)? }; + let cap: &Foo = unsafe { PyCapsule::import(py, name.as_ref())? }; assert_eq!(cap.val, 123); Ok(()) }) From 0bd03de142725733517f6c711e6bf914579ed812 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sat, 20 Nov 2021 18:00:22 +0100 Subject: [PATCH 17/35] Trying to figure out passing functions with T: Send [skip ci] --- src/pycapsule.rs | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index 4b7f71721cc..a986679d71b 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -46,14 +46,18 @@ pyobject_native_type_core!(PyCapsule, ffi::PyCapsule_Type, #checkfunction=ffi::P impl PyCapsule { /// Constructs a new capsule whose contents are `value`, associated with `name`. - pub fn new<'py, T: 'static>(py: Python<'py>, value: T, name: &CStr) -> PyResult<&'py Self> { + pub fn new<'py, T: 'static + Send>( + py: Python<'py>, + value: T, + name: &CStr, + ) -> PyResult<&'py Self> { Self::new_with_destructor(py, value, name, std::mem::drop) } /// Constructs a new capsule whose contents are `value`, associated with `name`. - /// + /// /// Also provides a destructor: when the `PyCapsule` is destroyed, it will be passed the original object. - pub fn new_with_destructor<'py, T: 'static, F: FnOnce(T)>( + pub fn new_with_destructor<'py, T: 'static + Send, F: FnOnce(T)>( py: Python<'py>, value: T, name: &CStr, @@ -89,7 +93,16 @@ impl PyCapsule { } /// Sets the context pointer in the capsule to `T`. - pub fn set_context(&self, py: Python, context: T) -> PyResult<()> { + /// + /// # Notes + /// + /// Context is not destructed like the value of the capsule is by `destructor`. Therefore, + /// it's likely this value will be leaked. If set, it's up to the user to either ignore this + /// side effect or figure another (unsafe) method of clean up. + /// + /// Context itself, is treated much like the value of the capsule, but should likely act as + /// a place to store any state managment when using the capsule. + pub fn set_context(&self, py: Python, context: T) -> PyResult<()> { let ctx = Box::new(context); let result = unsafe { ffi::PyCapsule_SetContext(self.as_ptr(), Box::into_raw(ctx) as _) as u8 }; @@ -197,13 +210,14 @@ mod tests { #[test] fn test_pycapsule_func() { - let cap: Py = Python::with_gil(|py| { - extern "C" fn foo(x: u32) -> u32 { - x - } + fn foo(x: u32) -> u32 { + x + } + + let cap: Py = Python::with_gil(|py| { let name = CString::new("foo").unwrap(); - let cap = PyCapsule::new(py, foo as *const c_void, &name).unwrap(); + let cap = PyCapsule::new(py, foo, &name).unwrap(); cap.into() }); @@ -211,6 +225,7 @@ mod tests { let f = unsafe { cap.as_ref(py).reference:: u32>() }; assert_eq!(f(123), 123); }); + panic!("Failed"); } #[test] @@ -248,8 +263,7 @@ mod tests { // check error when wrong named passed for capsule. let wrong_name = CString::new("builtins.non_existant").unwrap(); - let result: PyResult<&Foo> = - unsafe { PyCapsule::import(py, wrong_name.as_ref()) }; + let result: PyResult<&Foo> = unsafe { PyCapsule::import(py, wrong_name.as_ref()) }; assert!(result.is_err()); // corret name is okay. From 6072403cbd5a0e7f9157f94239f248b460aa58f6 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 11:46:28 +0100 Subject: [PATCH 18/35] Cleanup and add back casting of function value to PyCapsule --- src/pycapsule.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index a986679d71b..9fea0a8b907 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -210,14 +210,13 @@ mod tests { #[test] fn test_pycapsule_func() { - - fn foo(x: u32) -> u32 { - x - } - let cap: Py = Python::with_gil(|py| { + extern "C" fn foo(x: u32) -> u32 { + x + } + let name = CString::new("foo").unwrap(); - let cap = PyCapsule::new(py, foo, &name).unwrap(); + let cap = PyCapsule::new(py, foo as *const c_void, &name).unwrap(); cap.into() }); @@ -225,7 +224,6 @@ mod tests { let f = unsafe { cap.as_ref(py).reference:: u32>() }; assert_eq!(f(123), 123); }); - panic!("Failed"); } #[test] From 30a4e8ff63dafcfeaaf475fa40ee1a0b4f6fa4a9 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 12:18:18 +0100 Subject: [PATCH 19/35] Change destructor to FnOnce(T, *mut c_void) to handle potential context --- src/pycapsule.rs | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index 9fea0a8b907..0a8aae0010f 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -51,13 +51,14 @@ impl PyCapsule { value: T, name: &CStr, ) -> PyResult<&'py Self> { - Self::new_with_destructor(py, value, name, std::mem::drop) + Self::new_with_destructor(py, value, name, |_, _| {}) } /// Constructs a new capsule whose contents are `value`, associated with `name`. /// - /// Also provides a destructor: when the `PyCapsule` is destroyed, it will be passed the original object. - pub fn new_with_destructor<'py, T: 'static + Send, F: FnOnce(T)>( + /// Also provides a destructor: when the `PyCapsule` is destroyed, it will be passed the original object, + /// as well as `*mut c_void` which will point to the capsule's context, if any. + pub fn new_with_destructor<'py, T: 'static + Send, F: FnOnce(T, *mut c_void)>( py: Python<'py>, value: T, name: &CStr, @@ -162,23 +163,24 @@ impl PyCapsule { // C layout, as PyCapsule::get_reference depends on `T` being first. #[repr(C)] -struct CapsuleContents { +struct CapsuleContents { value: T, destructor: D, } // Wrapping ffi::PyCapsule_Destructor for a user supplied FnOnce(T) for capsule destructor -unsafe extern "C" fn capsule_destructor(capsule: *mut ffi::PyObject) { +unsafe extern "C" fn capsule_destructor(capsule: *mut ffi::PyObject) { let ptr = ffi::PyCapsule_GetPointer(capsule, ffi::PyCapsule_GetName(capsule)); + let ctx = ffi::PyCapsule_GetContext(capsule); let CapsuleContents { value, destructor } = *Box::from_raw(ptr as *mut CapsuleContents); - destructor(value) + destructor(value, ctx) } #[cfg(test)] mod tests { use crate::prelude::PyModule; use crate::{pycapsule::PyCapsule, Py, PyResult, Python}; - use std::ffi::{c_void, CString}; + use std::ffi::CString; #[test] fn test_pycapsule_struct() -> PyResult<()> { @@ -210,19 +212,20 @@ mod tests { #[test] fn test_pycapsule_func() { - let cap: Py = Python::with_gil(|py| { - extern "C" fn foo(x: u32) -> u32 { - x - } + + fn foo(x: u32) -> u32 { + x + } + let cap: Py = Python::with_gil(|py| { let name = CString::new("foo").unwrap(); - let cap = PyCapsule::new(py, foo as *const c_void, &name).unwrap(); + let cap = PyCapsule::new(py, foo, &name).unwrap(); cap.into() }); Python::with_gil(|py| { - let f = unsafe { cap.as_ref(py).reference:: u32>() }; - assert_eq!(f(123), 123); + //let f = unsafe { cap.as_ref(py).reference::<&fn(u32) -> u32>() }; + //assert_eq!(f(123), 123); }); } From f0cf17f566e5e3db6246146d897714d3a84a1edf Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 12:43:15 +0100 Subject: [PATCH 20/35] Assert capsule value size > 0 --- src/pycapsule.rs | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index 0a8aae0010f..9de7010aff5 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -3,6 +3,7 @@ use crate::Python; use crate::{ffi, AsPyPointer, PyAny}; use crate::{pyobject_native_type_core, PyErr, PyResult}; use std::ffi::{c_void, CStr}; +use std::mem::size_of; use std::os::raw::c_int; /// Represents a Python Capsule @@ -46,6 +47,10 @@ pyobject_native_type_core!(PyCapsule, ffi::PyCapsule_Type, #checkfunction=ffi::P impl PyCapsule { /// Constructs a new capsule whose contents are `value`, associated with `name`. + /// + /// # Notes + /// + /// An attempt to add a zero sized value will panic. pub fn new<'py, T: 'static + Send>( py: Python<'py>, value: T, @@ -58,12 +63,20 @@ impl PyCapsule { /// /// Also provides a destructor: when the `PyCapsule` is destroyed, it will be passed the original object, /// as well as `*mut c_void` which will point to the capsule's context, if any. + /// + /// # Notes + /// + /// An attempt to add a zero sized value will panic. pub fn new_with_destructor<'py, T: 'static + Send, F: FnOnce(T, *mut c_void)>( py: Python<'py>, value: T, name: &CStr, destructor: F, ) -> PyResult<&'py Self> { + assert!( + size_of::() > 0, + "Zero sized objects not allowed in capsule." + ); let val = Box::new(CapsuleContents { value, destructor }); let cap_ptr = unsafe { @@ -169,7 +182,9 @@ struct CapsuleContents { } // Wrapping ffi::PyCapsule_Destructor for a user supplied FnOnce(T) for capsule destructor -unsafe extern "C" fn capsule_destructor(capsule: *mut ffi::PyObject) { +unsafe extern "C" fn capsule_destructor( + capsule: *mut ffi::PyObject, +) { let ptr = ffi::PyCapsule_GetPointer(capsule, ffi::PyCapsule_GetName(capsule)); let ctx = ffi::PyCapsule_GetContext(capsule); let CapsuleContents { value, destructor } = *Box::from_raw(ptr as *mut CapsuleContents); @@ -212,20 +227,19 @@ mod tests { #[test] fn test_pycapsule_func() { - fn foo(x: u32) -> u32 { x } let cap: Py = Python::with_gil(|py| { let name = CString::new("foo").unwrap(); - let cap = PyCapsule::new(py, foo, &name).unwrap(); + let cap = PyCapsule::new(py, foo as fn(u32) -> u32, &name).unwrap(); cap.into() }); Python::with_gil(|py| { - //let f = unsafe { cap.as_ref(py).reference::<&fn(u32) -> u32>() }; - //assert_eq!(f(123), 123); + let f = unsafe { cap.as_ref(py).reference:: u32>() }; + assert_eq!(f(123), 123); }); } @@ -233,7 +247,7 @@ mod tests { fn test_pycapsule_context() -> PyResult<()> { Python::with_gil(|py| { let name = CString::new("foo").unwrap(); - let cap = PyCapsule::new(py, (), &name)?; + let cap = PyCapsule::new(py, 0, &name)?; let c = unsafe { cap.get_context::<()>(py)? }; assert!(c.is_none()); @@ -295,7 +309,7 @@ mod tests { fn test_vec_context() { let cap: Py = Python::with_gil(|py| { let name = CString::new("foo").unwrap(); - let cap = PyCapsule::new(py, (), &name).unwrap(); + let cap = PyCapsule::new(py, 0, &name).unwrap(); let ctx: Vec = vec![1, 2, 3, 4]; cap.set_context(py, ctx).unwrap(); From 5c0a22bbe31cce73bd8dd5f9803d3e0fd64fe1f7 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 13:15:43 +0100 Subject: [PATCH 21/35] Fixup: doc test fail fix --- src/pycapsule.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index 9de7010aff5..baf104c3b5c 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -34,7 +34,7 @@ use std::os::raw::c_int; /// let module = PyModule::import(py, "builtins")?; /// module.add("capsule", capsule)?; /// -/// let cap: &Foo = unsafe { PyCapsule::import(py, name.as_ref(), false)? }; +/// let cap: &Foo = unsafe { PyCapsule::import(py, name.as_ref())? }; /// assert_eq!(cap.val, 123); /// Ok(()) /// }); From f9fe8403f7504ee740329c65814722fe99ab3962 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 15:03:43 +0100 Subject: [PATCH 22/35] Fix merge mistake --- CHANGELOG.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc2a89d781d..521c64aef97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,6 @@ PyO3 versions, please see the [migration guide](https://pyo3.rs/latest/migration The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - ## Unreleased ### Packaging @@ -49,7 +48,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix build failure on PyPy when abi3 features are activated. [#1991](https://github.com/PyO3/pyo3/pull/1991) - Fix mingw platform detection. [#1993](https://github.com/PyO3/pyo3/pull/1993) - Fix panic in `__get__` implementation when accessing descriptor on type object. [#1997](https://github.com/PyO3/pyo3/pull/1997) ->>>>>>> main ## [0.15.0] - 2021-11-03 From 9d8394099eacba08ad96dad1307727b51b1e4dd9 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 15:49:36 +0100 Subject: [PATCH 23/35] Add destructor test and docs --- src/pycapsule.rs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index baf104c3b5c..50b8a71f63e 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -67,6 +67,14 @@ impl PyCapsule { /// # Notes /// /// An attempt to add a zero sized value will panic. + /// + /// # Example + /// + /// ``` + /// use pyo3::{PyCapsule}; + /// + /// + /// ``` pub fn new_with_destructor<'py, T: 'static + Send, F: FnOnce(T, *mut c_void)>( py: Python<'py>, value: T, @@ -193,9 +201,12 @@ unsafe extern "C" fn capsule_destructor PyResult<()> { @@ -322,4 +333,29 @@ mod tests { assert_eq!(ctx, Some(&vec![1_u8, 2, 3, 4])); }) } + + #[test] + fn test_pycapsule_destructor() { + + let (tx, rx) = channel(); + + fn destructor(_val: u32, ctx: *mut c_void) { + assert!(!ctx.is_null()); + let context = unsafe { *Box::from_raw(ctx as *mut Sender) }; + context.send(true).unwrap(); + } + + { + let _cap: Py = Python::with_gil(|py| { + let name = CString::new("foo").unwrap(); + let cap = PyCapsule::new_with_destructor(py, 0, &name, destructor).unwrap(); + cap.set_context(py, tx).unwrap(); + cap.into() + }); + } + + assert_eq!(rx.recv(), Ok(true)); + + + } } From 934e61b8eaf6f5272bb738b3d02e898bbced0690 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 16:02:24 +0100 Subject: [PATCH 24/35] fixup --- src/pycapsule.rs | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index 50b8a71f63e..fa6eb2ae3a7 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -67,14 +67,6 @@ impl PyCapsule { /// # Notes /// /// An attempt to add a zero sized value will panic. - /// - /// # Example - /// - /// ``` - /// use pyo3::{PyCapsule}; - /// - /// - /// ``` pub fn new_with_destructor<'py, T: 'static + Send, F: FnOnce(T, *mut c_void)>( py: Python<'py>, value: T, @@ -336,26 +328,22 @@ mod tests { #[test] fn test_pycapsule_destructor() { - - let (tx, rx) = channel(); + let (tx, rx) = channel::(); fn destructor(_val: u32, ctx: *mut c_void) { assert!(!ctx.is_null()); - let context = unsafe { *Box::from_raw(ctx as *mut Sender) }; + let context = unsafe { Box::from_raw(ctx as *mut Sender) }; context.send(true).unwrap(); } - - { - let _cap: Py = Python::with_gil(|py| { - let name = CString::new("foo").unwrap(); - let cap = PyCapsule::new_with_destructor(py, 0, &name, destructor).unwrap(); - cap.set_context(py, tx).unwrap(); - cap.into() - }); - } - - assert_eq!(rx.recv(), Ok(true)); + Python::with_gil(|py| { + let name = CString::new("foo").unwrap(); + let cap = + PyCapsule::new_with_destructor(py, 0, &name, destructor as fn(u32, *mut c_void)) + .unwrap(); + cap.set_context(py, tx).unwrap(); + }); + assert_eq!(rx.recv(), Ok(true)); } } From c13bbc9684376b0117aa37b6b3ff9fc5c31fb3bb Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 16:09:46 +0100 Subject: [PATCH 25/35] Fix docs --- src/pycapsule.rs | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index fa6eb2ae3a7..e8a682636ae 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -22,21 +22,21 @@ use std::os::raw::c_int; /// /// #[repr(C)] /// struct Foo { -/// pub val: u32, +/// pub val: u32, /// } /// /// let r = Python::with_gil(|py| -> PyResult<()> { -/// let foo = Foo { val: 123 }; -/// let name = CString::new("builtins.capsule").unwrap(); +/// let foo = Foo { val: 123 }; +/// let name = CString::new("builtins.capsule").unwrap(); /// -/// let capsule = PyCapsule::new(py, foo, name.as_ref())?; +/// let capsule = PyCapsule::new(py, foo, name.as_ref())?; /// -/// let module = PyModule::import(py, "builtins")?; -/// module.add("capsule", capsule)?; +/// let module = PyModule::import(py, "builtins")?; +/// module.add("capsule", capsule)?; /// -/// let cap: &Foo = unsafe { PyCapsule::import(py, name.as_ref())? }; -/// assert_eq!(cap.val, 123); -/// Ok(()) +/// let cap: &Foo = unsafe { PyCapsule::import(py, name.as_ref())? }; +/// assert_eq!(cap.val, 123); +/// Ok(()) /// }); /// assert!(r.is_ok()); /// ``` @@ -47,8 +47,10 @@ pyobject_native_type_core!(PyCapsule, ffi::PyCapsule_Type, #checkfunction=ffi::P impl PyCapsule { /// Constructs a new capsule whose contents are `value`, associated with `name`. + /// `name` is the identifier for the capsule; if it is stored as an attribute of a module, + /// the name should be in the format `modulename.attribute`. /// - /// # Notes + /// # Panics /// /// An attempt to add a zero sized value will panic. pub fn new<'py, T: 'static + Send>( @@ -64,7 +66,7 @@ impl PyCapsule { /// Also provides a destructor: when the `PyCapsule` is destroyed, it will be passed the original object, /// as well as `*mut c_void` which will point to the capsule's context, if any. /// - /// # Notes + /// # Panics /// /// An attempt to add a zero sized value will panic. pub fn new_with_destructor<'py, T: 'static + Send, F: FnOnce(T, *mut c_void)>( @@ -110,9 +112,11 @@ impl PyCapsule { /// /// # Notes /// - /// Context is not destructed like the value of the capsule is by `destructor`. Therefore, - /// it's likely this value will be leaked. If set, it's up to the user to either ignore this - /// side effect or figure another (unsafe) method of clean up. + /// The default destructor does not drop the context value. If you need to ensure it is not + /// leaked, use `new_with_destructor`, let it call `Box::from_raw()` with the correct type, + /// and drop it. + /// + /// Finally, if `set_context` is called twice in a row, the previous value is always leaked. /// /// Context itself, is treated much like the value of the capsule, but should likely act as /// a place to store any state managment when using the capsule. From 7a87a15760bedeeffb0fdaf47bc117a9f3548289 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 17:38:18 +0100 Subject: [PATCH 26/35] Fail to compile when zero sized type is used in PyCapsule value [skip ci] --- src/pycapsule.rs | 51 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 45 insertions(+), 6 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index e8a682636ae..55e5c93c23a 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -3,7 +3,6 @@ use crate::Python; use crate::{ffi, AsPyPointer, PyAny}; use crate::{pyobject_native_type_core, PyErr, PyResult}; use std::ffi::{c_void, CStr}; -use std::mem::size_of; use std::os::raw::c_int; /// Represents a Python Capsule @@ -53,6 +52,32 @@ impl PyCapsule { /// # Panics /// /// An attempt to add a zero sized value will panic. + /// + /// # Example + /// + /// ``` + /// use pyo3::{prelude::*, pycapsule::PyCapsule}; + /// use std::ffi::CString; + /// + /// Python::with_gil(|py| { + /// let name = CString::new("foo").unwrap(); + /// let capsule = PyCapsule::new(py, 123_u32, &name).unwrap(); + /// let val = unsafe { capsule.reference::() }; + /// assert_eq!(*val, 123); + /// }); + /// ``` + /// + /// However, attempting to construct a `PyCapsule` with a zero sized type will not compile: + /// + /// ```compile_fail + /// use pyo3::{prelude::*, pycapsule::PyCapsule}; + /// use std::ffi::CString; + /// + /// Python::with_gil(|py| { + /// let name = CString::new("foo").unwrap(); + /// let capsule = PyCapsule::new(py, (), &name).unwrap(); // Oops! `()` is zero sized! + /// }); + /// ``` pub fn new<'py, T: 'static + Send>( py: Python<'py>, value: T, @@ -69,16 +94,17 @@ impl PyCapsule { /// # Panics /// /// An attempt to add a zero sized value will panic. - pub fn new_with_destructor<'py, T: 'static + Send, F: FnOnce(T, *mut c_void)>( + pub fn new_with_destructor< + 'py, + T: 'static + Send + PanicWhenZeroSized, + F: FnOnce(T, *mut c_void), + >( py: Python<'py>, value: T, name: &CStr, destructor: F, ) -> PyResult<&'py Self> { - assert!( - size_of::() > 0, - "Zero sized objects not allowed in capsule." - ); + PanicWhenZeroSized::assert_not_zero_sized(&value); let val = Box::new(CapsuleContents { value, destructor }); let cap_ptr = unsafe { @@ -195,6 +221,19 @@ unsafe extern "C" fn capsule_destructor() == 0) as usize; + const _CHECK: &'static str = ["type must not be zero-sized!"][Self::_CONDITION]; + #[allow(path_statements, clippy::no_effect)] + fn assert_not_zero_sized(&self) { + ::_CHECK; + } +} + +impl PanicWhenZeroSized for T {} + #[cfg(test)] mod tests { use libc::c_void; From 73ad0d15af366d290945e8029d5f57d5efb50fd1 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 17:42:07 +0100 Subject: [PATCH 27/35] Fixup: doc fix for PanicWhenZeroSized [skip ci] --- src/pycapsule.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index 55e5c93c23a..c9f7ffd3797 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -221,8 +221,8 @@ unsafe extern "C" fn capsule_destructor() == 0) as usize; const _CHECK: &'static str = ["type must not be zero-sized!"][Self::_CONDITION]; From e43528485d5b6ab37212f369a5b29e0dc224fe11 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 18:14:06 +0100 Subject: [PATCH 28/35] Make context always *mut void w/ examples --- src/pycapsule.rs | 89 +++++++++++++++++++++++++++++++----------------- 1 file changed, 57 insertions(+), 32 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index c9f7ffd3797..cba079409ad 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -134,22 +134,48 @@ impl PyCapsule { } } - /// Sets the context pointer in the capsule to `T`. + /// Sets the context pointer in the capsule. /// /// # Notes /// /// The default destructor does not drop the context value. If you need to ensure it is not /// leaked, use `new_with_destructor`, let it call `Box::from_raw()` with the correct type, - /// and drop it. + /// and drop it. (See example.) /// /// Finally, if `set_context` is called twice in a row, the previous value is always leaked. /// /// Context itself, is treated much like the value of the capsule, but should likely act as /// a place to store any state managment when using the capsule. - pub fn set_context(&self, py: Python, context: T) -> PyResult<()> { - let ctx = Box::new(context); - let result = - unsafe { ffi::PyCapsule_SetContext(self.as_ptr(), Box::into_raw(ctx) as _) as u8 }; + /// + /// # Example + /// + /// ``` + /// use std::ffi::CString; + /// use std::sync::mpsc::{channel, Sender}; + /// use libc::c_void; + /// use pyo3::{prelude::*, pycapsule::PyCapsule}; + /// + /// let (tx, rx) = channel::(); + /// + /// fn destructor(val: u32, context: *mut c_void) { + /// let ctx = unsafe { *Box::from_raw(context as *mut Sender) }; + /// ctx.send("Destructor called!".to_string()).unwrap(); + /// } + /// + /// Python::with_gil(|py| { + /// let name = CString::new("foo").unwrap(); + /// let capsule = + /// PyCapsule::new_with_destructor(py, 123, &name, destructor as fn(u32, *mut c_void)) + /// .unwrap(); + /// let context = Box::new(tx); // `Sender` is our context, box it up and ship it! + /// capsule.set_context(py, Box::into_raw(context) as *mut c_void).unwrap(); + /// // This scope will end, causing our destructor to be called... + /// }); + /// + /// assert_eq!(rx.recv(), Ok("Destructor called!".to_string())); + /// ``` + pub fn set_context(&self, py: Python, context: *mut c_void) -> PyResult<()> { + let result = unsafe { ffi::PyCapsule_SetContext(self.as_ptr(), context) as u8 }; if result != 0 { Err(PyErr::fetch(py)) } else { @@ -157,21 +183,14 @@ impl PyCapsule { } } - /// Gets a reference to the context of the capsule, if any. - /// - /// # Safety - /// - /// It must be known that this capsule contains a context of type `T`. - pub unsafe fn get_context(&self, py: Python) -> PyResult> { - let ctx = ffi::PyCapsule_GetContext(self.as_ptr()); - if ctx.is_null() { - if self.is_valid() && PyErr::occurred(py) { - Err(PyErr::fetch(py)) - } else { - Ok(None) - } + /// Gets the current context stored in the capsule. If there is no context, the pointer + /// will be null. + pub fn get_context(&self, py: Python) -> PyResult<*mut c_void> { + let ctx = unsafe { ffi::PyCapsule_GetContext(self.as_ptr()) }; + if ctx.is_null() && self.is_valid() && PyErr::occurred(py) { + Err(PyErr::fetch(py)) } else { - Ok(Some(&*(ctx as *const T))) + Ok(ctx) } } @@ -295,13 +314,15 @@ mod tests { let name = CString::new("foo").unwrap(); let cap = PyCapsule::new(py, 0, &name)?; - let c = unsafe { cap.get_context::<()>(py)? }; - assert!(c.is_none()); + let c = cap.get_context(py)?; + assert!(c.is_null()); - cap.set_context(py, 123)?; + let ctx = Box::new(123_u32); + cap.set_context(py, Box::into_raw(ctx) as _)?; - let ctx: Option<&u32> = unsafe { cap.get_context(py)? }; - assert_eq!(ctx, Some(&123)); + let ctx_ptr: *mut c_void = cap.get_context(py)?; + let ctx = unsafe { *Box::from_raw(ctx_ptr as *mut u32) }; + assert_eq!(ctx, 123); Ok(()) }) } @@ -353,19 +374,21 @@ mod tests { #[test] fn test_vec_context() { + let context: Vec = vec![1, 2, 3, 4]; + let cap: Py = Python::with_gil(|py| { let name = CString::new("foo").unwrap(); let cap = PyCapsule::new(py, 0, &name).unwrap(); - - let ctx: Vec = vec![1, 2, 3, 4]; - cap.set_context(py, ctx).unwrap(); + cap.set_context(py, Box::into_raw(Box::new(&context)) as _) + .unwrap(); cap.into() }); Python::with_gil(|py| { - let ctx: Option<&Vec> = unsafe { cap.as_ref(py).get_context(py).unwrap() }; - assert_eq!(ctx, Some(&vec![1_u8, 2, 3, 4])); + let ctx_ptr: *mut c_void = cap.as_ref(py).get_context(py).unwrap(); + let ctx = unsafe { *Box::from_raw(ctx_ptr as *mut &Vec) }; + assert_eq!(ctx, &vec![1_u8, 2, 3, 4]); }) } @@ -375,7 +398,7 @@ mod tests { fn destructor(_val: u32, ctx: *mut c_void) { assert!(!ctx.is_null()); - let context = unsafe { Box::from_raw(ctx as *mut Sender) }; + let context = unsafe { *Box::from_raw(ctx as *mut Sender) }; context.send(true).unwrap(); } @@ -384,9 +407,11 @@ mod tests { let cap = PyCapsule::new_with_destructor(py, 0, &name, destructor as fn(u32, *mut c_void)) .unwrap(); - cap.set_context(py, tx).unwrap(); + cap.set_context(py, Box::into_raw(Box::new(tx)) as _) + .unwrap(); }); + // the destructor was called. assert_eq!(rx.recv(), Ok(true)); } } From d0c623d1678c2f35ee7ca6df6c13f1f3655bc52e Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 18:23:26 +0100 Subject: [PATCH 29/35] Fix clippy and docs --- src/pycapsule.rs | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index cba079409ad..a0043d59de4 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -147,6 +147,10 @@ impl PyCapsule { /// Context itself, is treated much like the value of the capsule, but should likely act as /// a place to store any state managment when using the capsule. /// + /// # Safety + /// + /// This might dereference the raw pointer `context`, which is unsafe. + /// /// # Example /// /// ``` @@ -168,14 +172,14 @@ impl PyCapsule { /// PyCapsule::new_with_destructor(py, 123, &name, destructor as fn(u32, *mut c_void)) /// .unwrap(); /// let context = Box::new(tx); // `Sender` is our context, box it up and ship it! - /// capsule.set_context(py, Box::into_raw(context) as *mut c_void).unwrap(); + /// unsafe { capsule.set_context(py, Box::into_raw(context) as *mut c_void).unwrap() }; /// // This scope will end, causing our destructor to be called... /// }); /// /// assert_eq!(rx.recv(), Ok("Destructor called!".to_string())); /// ``` - pub fn set_context(&self, py: Python, context: *mut c_void) -> PyResult<()> { - let result = unsafe { ffi::PyCapsule_SetContext(self.as_ptr(), context) as u8 }; + pub unsafe fn set_context(&self, py: Python, context: *mut c_void) -> PyResult<()> { + let result = ffi::PyCapsule_SetContext(self.as_ptr(), context) as u8; if result != 0 { Err(PyErr::fetch(py)) } else { @@ -241,7 +245,7 @@ unsafe extern "C" fn capsule_destructor` pub trait PanicWhenZeroSized: Sized { const _CONDITION: usize = (std::mem::size_of::() == 0) as usize; const _CHECK: &'static str = ["type must not be zero-sized!"][Self::_CONDITION]; @@ -318,7 +322,7 @@ mod tests { assert!(c.is_null()); let ctx = Box::new(123_u32); - cap.set_context(py, Box::into_raw(ctx) as _)?; + unsafe { cap.set_context(py, Box::into_raw(ctx) as _)? }; let ctx_ptr: *mut c_void = cap.get_context(py)?; let ctx = unsafe { *Box::from_raw(ctx_ptr as *mut u32) }; @@ -379,8 +383,10 @@ mod tests { let cap: Py = Python::with_gil(|py| { let name = CString::new("foo").unwrap(); let cap = PyCapsule::new(py, 0, &name).unwrap(); - cap.set_context(py, Box::into_raw(Box::new(&context)) as _) - .unwrap(); + unsafe { + cap.set_context(py, Box::into_raw(Box::new(&context)) as _) + .unwrap() + }; cap.into() }); @@ -407,8 +413,10 @@ mod tests { let cap = PyCapsule::new_with_destructor(py, 0, &name, destructor as fn(u32, *mut c_void)) .unwrap(); - cap.set_context(py, Box::into_raw(Box::new(tx)) as _) - .unwrap(); + unsafe { + cap.set_context(py, Box::into_raw(Box::new(tx)) as _) + .unwrap() + }; }); // the destructor was called. From 1962a652693cfb38a081a5aeb3d1cd5c8e9a4d95 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 19:49:12 +0100 Subject: [PATCH 30/35] Address PR comments (docs & zero size check refactor) [skip ci] --- src/pycapsule.rs | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index a0043d59de4..88b20a23d76 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -49,9 +49,8 @@ impl PyCapsule { /// `name` is the identifier for the capsule; if it is stored as an attribute of a module, /// the name should be in the format `modulename.attribute`. /// - /// # Panics - /// - /// An attempt to add a zero sized value will panic. + /// It is checked at compile time that the type T is not zero-sized. Rust function items + /// need to cast to a function pointer (`fn(args) -> result`) to be put into a capsule. /// /// # Example /// @@ -78,7 +77,7 @@ impl PyCapsule { /// let capsule = PyCapsule::new(py, (), &name).unwrap(); // Oops! `()` is zero sized! /// }); /// ``` - pub fn new<'py, T: 'static + Send>( + pub fn new<'py, T: 'static + Send + AssertNotZeroSized>( py: Python<'py>, value: T, name: &CStr, @@ -90,13 +89,9 @@ impl PyCapsule { /// /// Also provides a destructor: when the `PyCapsule` is destroyed, it will be passed the original object, /// as well as `*mut c_void` which will point to the capsule's context, if any. - /// - /// # Panics - /// - /// An attempt to add a zero sized value will panic. pub fn new_with_destructor< 'py, - T: 'static + Send + PanicWhenZeroSized, + T: 'static + Send + AssertNotZeroSized, F: FnOnce(T, *mut c_void), >( py: Python<'py>, @@ -104,7 +99,7 @@ impl PyCapsule { name: &CStr, destructor: F, ) -> PyResult<&'py Self> { - PanicWhenZeroSized::assert_not_zero_sized(&value); + AssertNotZeroSized::assert_not_zero_sized(&value); let val = Box::new(CapsuleContents { value, destructor }); let cap_ptr = unsafe { @@ -138,15 +133,14 @@ impl PyCapsule { /// /// # Notes /// - /// The default destructor does not drop the context value. If you need to ensure it is not - /// leaked, use `new_with_destructor`, let it call `Box::from_raw()` with the correct type, - /// and drop it. (See example.) - /// - /// Finally, if `set_context` is called twice in a row, the previous value is always leaked. - /// /// Context itself, is treated much like the value of the capsule, but should likely act as /// a place to store any state managment when using the capsule. /// + /// If you want to store a Rust value as the context, and drop it from the destructor, use + /// `Box::into_raw` to convert it into a pointer, see the example. + /// + /// Finally, if `set_context` is called twice in a row, the previous value is always leaked. + /// /// # Safety /// /// This might dereference the raw pointer `context`, which is unsafe. @@ -246,16 +240,17 @@ unsafe extern "C" fn capsule_destructor` -pub trait PanicWhenZeroSized: Sized { +pub trait AssertNotZeroSized: Sized { const _CONDITION: usize = (std::mem::size_of::() == 0) as usize; - const _CHECK: &'static str = ["type must not be zero-sized!"][Self::_CONDITION]; + const _CHECK: &'static str = + ["PyCapsule value type T must not be zero-sized!"][Self::_CONDITION]; #[allow(path_statements, clippy::no_effect)] fn assert_not_zero_sized(&self) { - ::_CHECK; + ::_CHECK; } } -impl PanicWhenZeroSized for T {} +impl AssertNotZeroSized for T {} #[cfg(test)] mod tests { From ba3f15e64a0e3a3de17c31fd3c0c3cb4b238c759 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 19:50:25 +0100 Subject: [PATCH 31/35] Hide docs for AssertNotZeroSized trait [skip ci] --- src/pycapsule.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index 88b20a23d76..a43307a65bb 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -240,6 +240,7 @@ unsafe extern "C" fn capsule_destructor` +#[doc(hidden)] pub trait AssertNotZeroSized: Sized { const _CONDITION: usize = (std::mem::size_of::() == 0) as usize; const _CHECK: &'static str = From 3662a49b76b52cee909217bab4a10f236f8bce78 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 19:58:14 +0100 Subject: [PATCH 32/35] Remove unsafe from PyCapsule::set_context (ignore clippy warning) --- src/pycapsule.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index a43307a65bb..dd58e0cb1d9 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -166,14 +166,15 @@ impl PyCapsule { /// PyCapsule::new_with_destructor(py, 123, &name, destructor as fn(u32, *mut c_void)) /// .unwrap(); /// let context = Box::new(tx); // `Sender` is our context, box it up and ship it! - /// unsafe { capsule.set_context(py, Box::into_raw(context) as *mut c_void).unwrap() }; + /// capsule.set_context(py, Box::into_raw(context) as *mut c_void).unwrap(); /// // This scope will end, causing our destructor to be called... /// }); /// /// assert_eq!(rx.recv(), Ok("Destructor called!".to_string())); /// ``` - pub unsafe fn set_context(&self, py: Python, context: *mut c_void) -> PyResult<()> { - let result = ffi::PyCapsule_SetContext(self.as_ptr(), context) as u8; + #[allow(clippy::not_unsafe_ptr_arg_deref)] + pub fn set_context(&self, py: Python, context: *mut c_void) -> PyResult<()> { + let result = unsafe { ffi::PyCapsule_SetContext(self.as_ptr(), context) as u8 }; if result != 0 { Err(PyErr::fetch(py)) } else { @@ -318,7 +319,7 @@ mod tests { assert!(c.is_null()); let ctx = Box::new(123_u32); - unsafe { cap.set_context(py, Box::into_raw(ctx) as _)? }; + cap.set_context(py, Box::into_raw(ctx) as _)?; let ctx_ptr: *mut c_void = cap.get_context(py)?; let ctx = unsafe { *Box::from_raw(ctx_ptr as *mut u32) }; @@ -379,10 +380,8 @@ mod tests { let cap: Py = Python::with_gil(|py| { let name = CString::new("foo").unwrap(); let cap = PyCapsule::new(py, 0, &name).unwrap(); - unsafe { - cap.set_context(py, Box::into_raw(Box::new(&context)) as _) - .unwrap() - }; + cap.set_context(py, Box::into_raw(Box::new(&context)) as _) + .unwrap(); cap.into() }); @@ -409,10 +408,8 @@ mod tests { let cap = PyCapsule::new_with_destructor(py, 0, &name, destructor as fn(u32, *mut c_void)) .unwrap(); - unsafe { - cap.set_context(py, Box::into_raw(Box::new(tx)) as _) - .unwrap() - }; + cap.set_context(py, Box::into_raw(Box::new(tx)) as _) + .unwrap(); }); // the destructor was called. From 14a3c09d569bbb961e4db9df43178fc35cf2b07b Mon Sep 17 00:00:00 2001 From: milesgranger Date: Sun, 21 Nov 2021 19:59:53 +0100 Subject: [PATCH 33/35] Remove safety docs from PyCapsule::set_context --- src/pycapsule.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index dd58e0cb1d9..4ce13492ab2 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -141,10 +141,6 @@ impl PyCapsule { /// /// Finally, if `set_context` is called twice in a row, the previous value is always leaked. /// - /// # Safety - /// - /// This might dereference the raw pointer `context`, which is unsafe. - /// /// # Example /// /// ``` From 4f2c9b84def9f247a33113a434bcc0e726547c17 Mon Sep 17 00:00:00 2001 From: milesgranger Date: Mon, 22 Nov 2021 08:17:32 +0100 Subject: [PATCH 34/35] Rename PyCapsule::get_pointer -> PyCapsule::pointer --- src/pycapsule.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pycapsule.rs b/src/pycapsule.rs index 4ce13492ab2..19a11f144c8 100644 --- a/src/pycapsule.rs +++ b/src/pycapsule.rs @@ -195,11 +195,11 @@ impl PyCapsule { /// /// It must be known that this capsule's pointer is to an item of type `T`. pub unsafe fn reference(&self) -> &T { - &*(self.get_pointer() as *const T) + &*(self.pointer() as *const T) } /// Gets the raw `c_void` pointer to the value in this capsule. - pub fn get_pointer(&self) -> *mut c_void { + pub fn pointer(&self) -> *mut c_void { unsafe { ffi::PyCapsule_GetPointer(self.0.as_ptr(), self.name().as_ptr()) } } From f904acfb2375c14ac4077c2784458bbcd6b51d08 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Mon, 22 Nov 2021 20:56:19 +0100 Subject: [PATCH 35/35] capsule: rename to types/capsule and make slight copyedits to the docstring. --- src/lib.rs | 1 - src/{pycapsule.rs => types/capsule.rs} | 28 ++++++++++++-------------- src/types/mod.rs | 2 ++ 3 files changed, 15 insertions(+), 16 deletions(-) rename src/{pycapsule.rs => types/capsule.rs} (92%) diff --git a/src/lib.rs b/src/lib.rs index dee7f9394ea..7d7d44b7dcb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -334,7 +334,6 @@ pub mod marshal; pub mod once_cell; pub mod panic; pub mod prelude; -pub mod pycapsule; pub mod pycell; pub mod pyclass; pub mod pyclass_init; diff --git a/src/pycapsule.rs b/src/types/capsule.rs similarity index 92% rename from src/pycapsule.rs rename to src/types/capsule.rs index 19a11f144c8..931a5355b17 100644 --- a/src/pycapsule.rs +++ b/src/types/capsule.rs @@ -17,7 +17,7 @@ use std::os::raw::c_int; /// # Example /// ``` /// use std::ffi::CString; -/// use pyo3::{prelude::*, pycapsule::PyCapsule}; +/// use pyo3::{prelude::*, types::PyCapsule}; /// /// #[repr(C)] /// struct Foo { @@ -47,15 +47,15 @@ pyobject_native_type_core!(PyCapsule, ffi::PyCapsule_Type, #checkfunction=ffi::P impl PyCapsule { /// Constructs a new capsule whose contents are `value`, associated with `name`. /// `name` is the identifier for the capsule; if it is stored as an attribute of a module, - /// the name should be in the format `modulename.attribute`. + /// the name should be in the format `"modulename.attribute"`. /// /// It is checked at compile time that the type T is not zero-sized. Rust function items - /// need to cast to a function pointer (`fn(args) -> result`) to be put into a capsule. + /// need to be cast to a function pointer (`fn(args) -> result`) to be put into a capsule. /// /// # Example /// /// ``` - /// use pyo3::{prelude::*, pycapsule::PyCapsule}; + /// use pyo3::{prelude::*, types::PyCapsule}; /// use std::ffi::CString; /// /// Python::with_gil(|py| { @@ -66,10 +66,10 @@ impl PyCapsule { /// }); /// ``` /// - /// However, attempting to construct a `PyCapsule` with a zero sized type will not compile: + /// However, attempting to construct a `PyCapsule` with a zero-sized type will not compile: /// /// ```compile_fail - /// use pyo3::{prelude::*, pycapsule::PyCapsule}; + /// use pyo3::{prelude::*, types::PyCapsule}; /// use std::ffi::CString; /// /// Python::with_gil(|py| { @@ -88,7 +88,7 @@ impl PyCapsule { /// Constructs a new capsule whose contents are `value`, associated with `name`. /// /// Also provides a destructor: when the `PyCapsule` is destroyed, it will be passed the original object, - /// as well as `*mut c_void` which will point to the capsule's context, if any. + /// as well as a `*mut c_void` which will point to the capsule's context, if any. pub fn new_with_destructor< 'py, T: 'static + Send + AssertNotZeroSized, @@ -115,11 +115,11 @@ impl PyCapsule { /// Imports an existing capsule. /// /// The `name` should match the path to the module attribute exactly in the form - /// of `module.attribute`, which should be the same as the name within the capsule. + /// of `"module.attribute"`, which should be the same as the name within the capsule. /// /// # Safety /// - /// It must be known that this capsule's pointer is to an item of type `T`. + /// It must be known that this capsule's value pointer is to an item of type `T`. pub unsafe fn import<'py, T>(py: Python<'py>, name: &CStr) -> PyResult<&'py T> { let ptr = ffi::PyCapsule_Import(name.as_ptr(), false as c_int); if ptr.is_null() { @@ -133,21 +133,19 @@ impl PyCapsule { /// /// # Notes /// - /// Context itself, is treated much like the value of the capsule, but should likely act as - /// a place to store any state managment when using the capsule. + /// The context is treated much like the value of the capsule, but should likely act as + /// a place to store any state management when using the capsule. /// /// If you want to store a Rust value as the context, and drop it from the destructor, use /// `Box::into_raw` to convert it into a pointer, see the example. /// - /// Finally, if `set_context` is called twice in a row, the previous value is always leaked. - /// /// # Example /// /// ``` /// use std::ffi::CString; /// use std::sync::mpsc::{channel, Sender}; /// use libc::c_void; - /// use pyo3::{prelude::*, pycapsule::PyCapsule}; + /// use pyo3::{prelude::*, types::PyCapsule}; /// /// let (tx, rx) = channel::(); /// @@ -255,7 +253,7 @@ mod tests { use libc::c_void; use crate::prelude::PyModule; - use crate::{pycapsule::PyCapsule, Py, PyResult, Python}; + use crate::{types::PyCapsule, Py, PyResult, Python}; use std::ffi::CString; use std::sync::mpsc::{channel, Sender}; diff --git a/src/types/mod.rs b/src/types/mod.rs index f670a32db92..cbbeafb51fe 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -6,6 +6,7 @@ pub use self::any::PyAny; pub use self::boolobject::PyBool; pub use self::bytearray::PyByteArray; pub use self::bytes::PyBytes; +pub use self::capsule::PyCapsule; pub use self::complex::PyComplex; #[cfg(not(Py_LIMITED_API))] pub use self::datetime::{ @@ -223,6 +224,7 @@ mod any; mod boolobject; mod bytearray; mod bytes; +mod capsule; mod complex; #[cfg(not(Py_LIMITED_API))] mod datetime;