misc: tidy ups pre 0.16

This commit is contained in:
David Hewitt 2022-02-26 22:18:28 +00:00
parent 1ed9a73f0f
commit b59ee9b54b
6 changed files with 153 additions and 49 deletions

View file

@ -105,6 +105,55 @@ Because there is no such distinction from Python, implementing these methods wil
The PyO3 behavior in 0.16 has been changed to be closer to this Python behavior by default.
### `wrap_pymodule!` now respects privacy correctly
Prior to PyO3 0.16 the `wrap_pymodule!` macro could use modules declared in Rust modules which were not reachable.
For example, the following code was legal before 0.16, but in 0.16 is rejected because the `wrap_pymodule!` macro cannot access the `private_submodule` function:
```rust,compile_fail
mod foo {
use pyo3::prelude::*;
#[pymodule]
fn private_submodule(_py: Python, m: &PyModule) -> PyResult<()> {
Ok(())
}
}
use pyo3::prelude::*;
use foo::*;
#[pymodule]
fn my_module(_py: Python, m: &PyModule) -> PyResult<()> {
m.add_wrapped(wrap_pymodule!(private_submodule))?;
Ok(())
}
```
To fix it, make the private submodule visible, e.g. with `pub` or `pub(crate)`.
```rust
mod foo {
use pyo3::prelude::*;
#[pymodule]
pub(crate) fn private_submodule(_py: Python, m: &PyModule) -> PyResult<()> {
Ok(())
}
}
use pyo3::prelude::*;
use pyo3::wrap_pymodule;
use foo::*;
#[pymodule]
fn my_module(_py: Python, m: &PyModule) -> PyResult<()> {
m.add_wrapped(wrap_pymodule!(private_submodule))?;
Ok(())
}
```
## from 0.14.* to 0.15
### Changes in sequence indexing

View file

@ -324,7 +324,7 @@ fn impl_traverse_slot(cls: &syn::Type, spec: FnSpec) -> Result<TokenStream> {
let visit = _pyo3::class::gc::PyVisit::from_raw(visit, arg, py);
let borrow = slf.try_borrow();
if let ::std::result::Result::Ok(borrow) = borrow {
_pyo3::class::gc::unwrap_traverse_result(borrow.#ident(visit))
_pyo3::impl_::pymethods::unwrap_traverse_result(borrow.#ident(visit))
} else {
0
}

View file

@ -3,11 +3,10 @@
//! Python GC support
use crate::{ffi, AsPyPointer, PyCell, PyClass, Python};
use crate::{ffi, PyCell, PyClass};
use std::os::raw::{c_int, c_void};
#[repr(transparent)]
pub struct PyTraverseError(c_int);
pub use crate::impl_::pymethods::{PyTraverseError, PyVisit};
/// GC support
#[deprecated(since = "0.16.0", note = "prefer `#[pymethods]` to `#[pyproto]`")]
@ -60,45 +59,3 @@ where
slf.borrow_mut().__clear__();
0
}
/// Object visitor for GC.
#[derive(Clone)]
pub struct PyVisit<'p> {
visit: ffi::visitproc,
arg: *mut c_void,
/// VisitProc contains a Python instance to ensure that
/// 1) it is cannot be moved out of the traverse() call
/// 2) it cannot be sent to other threads
_py: Python<'p>,
}
impl<'p> PyVisit<'p> {
/// Visit `obj`.
pub fn call<T>(&self, obj: &T) -> Result<(), PyTraverseError>
where
T: AsPyPointer,
{
let r = unsafe { (self.visit)(obj.as_ptr(), self.arg) };
if r == 0 {
Ok(())
} else {
Err(PyTraverseError(r))
}
}
/// Creates the PyVisit from the arguments to tp_traverse
#[doc(hidden)]
pub unsafe fn from_raw(visit: ffi::visitproc, arg: *mut c_void, _py: Python<'p>) -> Self {
Self { visit, arg, _py }
}
}
/// Unwraps the result of __traverse__ for tp_traverse
#[doc(hidden)]
#[inline]
pub fn unwrap_traverse_result(result: Result<(), PyTraverseError>) -> c_int {
match result {
Ok(()) => 0,
Err(PyTraverseError(value)) => value,
}
}

View file

@ -1,8 +1,8 @@
use crate::internal_tricks::{extract_cstr_or_leak_cstring, NulByteInString};
use crate::{ffi, FromPyObject, PyAny, PyObject, PyResult, Python};
use crate::{ffi, AsPyPointer, FromPyObject, PyAny, PyObject, PyResult, Python};
use std::ffi::CStr;
use std::fmt;
use std::os::raw::c_int;
use std::os::raw::{c_int, c_void};
/// Python 3.8 and up - __ipow__ has modulo argument correctly populated.
#[cfg(Py_3_8)]
@ -252,3 +252,48 @@ fn get_name(name: &'static str) -> Result<&'static CStr, NulByteInString> {
fn get_doc(doc: &'static str) -> Result<&'static CStr, NulByteInString> {
extract_cstr_or_leak_cstring(doc, "Document cannot contain NUL byte.")
}
#[repr(transparent)]
pub struct PyTraverseError(pub(crate) c_int);
/// Object visitor for GC.
#[derive(Clone)]
pub struct PyVisit<'p> {
pub(crate) visit: ffi::visitproc,
pub(crate) arg: *mut c_void,
/// VisitProc contains a Python instance to ensure that
/// 1) it is cannot be moved out of the traverse() call
/// 2) it cannot be sent to other threads
pub(crate) _py: Python<'p>,
}
impl<'p> PyVisit<'p> {
/// Visit `obj`.
pub fn call<T>(&self, obj: &T) -> Result<(), PyTraverseError>
where
T: AsPyPointer,
{
let r = unsafe { (self.visit)(obj.as_ptr(), self.arg) };
if r == 0 {
Ok(())
} else {
Err(PyTraverseError(r))
}
}
/// Creates the PyVisit from the arguments to tp_traverse
#[doc(hidden)]
pub unsafe fn from_raw(visit: ffi::visitproc, arg: *mut c_void, _py: Python<'p>) -> Self {
Self { visit, arg, _py }
}
}
/// Unwraps the result of __traverse__ for tp_traverse
#[doc(hidden)]
#[inline]
pub fn unwrap_traverse_result(result: Result<(), PyTraverseError>) -> c_int {
match result {
Ok(()) => 0,
Err(PyTraverseError(value)) => value,
}
}

View file

@ -62,7 +62,10 @@
//! ## Default feature flags
//!
//! The following features are turned on by default:
//! - `macros`: Enables various macros, including all the attribute macros.
//! - `macros`: Enables various macros, including all the attribute macros excluding the deprecated
//! `#[pyproto]` attribute.
//! - `pyproto`: Adds the deprecated `#[pyproto]` attribute macro. Likely to become optional and
//! then removed in the future.
//!
//! ## Optional feature flags
//!
@ -306,6 +309,8 @@ pub mod class {
#[doc(hidden)]
pub use crate::impl_::pymethods as methods;
pub use self::gc::{PyTraverseError, PyVisit};
#[doc(hidden)]
pub use self::methods::{
PyClassAttributeDef, PyGetterDef, PyMethodDef, PyMethodDefType, PyMethodType, PySetterDef,
@ -322,6 +327,10 @@ pub mod class {
pub mod iter {
pub use crate::pyclass::{IterNextOutput, PyIterNextOutput};
}
pub mod gc {
pub use crate::impl_::pymethods::{PyTraverseError, PyVisit};
}
}
#[cfg(feature = "macros")]

View file

@ -278,3 +278,47 @@ fn gc_during_borrow() {
drop(guard);
}
}
#[pyclass]
struct PanickyTraverse {
member: PyObject,
}
impl PanickyTraverse {
fn new(py: Python) -> Self {
Self { member: py.None() }
}
}
#[pymethods]
impl PanickyTraverse {
fn __traverse__(&self, visit: PyVisit) -> Result<(), PyTraverseError> {
visit.call(&self.member)?;
// In the test, we expect this to never be hit
unreachable!()
}
}
#[test]
fn traverse_error() {
Python::with_gil(|py| unsafe {
// declare a visitor function which errors (returns nonzero code)
extern "C" fn visit_error(
_object: *mut pyo3::ffi::PyObject,
_arg: *mut core::ffi::c_void,
) -> std::os::raw::c_int {
-1
}
// get the traverse function
let ty = PanickyTraverse::type_object(py).as_type_ptr();
let traverse = get_type_traverse(ty).unwrap();
// confirm that traversing errors
let obj = Py::new(py, PanickyTraverse::new(py)).unwrap();
assert_eq!(
traverse(obj.as_ptr(), visit_error, std::ptr::null_mut()),
-1
);
})
}