Skip to content

Commit

Permalink
kill off panic_result_into_callback_output
Browse files Browse the repository at this point in the history
  • Loading branch information
@davidhewitt
davidhewitt committed Oct 23, 2022
1 parent 2f25361 commit 9032f75
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 181 deletions.
2 changes: 1 addition & 1 deletion pyo3-macros-backend/src/module.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub fn pymodule_impl(
/// the module.
#[export_name = #pyinit_symbol]
pub unsafe extern "C" fn init() -> *mut #krate::ffi::PyObject {
DEF.module_init()
#krate::impl_::trampoline::module_init(|py| DEF.make_module(py))
}
}

Expand Down
22 changes: 0 additions & 22 deletions src/callback.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@
use crate::err::{PyErr, PyResult};
use crate::exceptions::PyOverflowError;
use crate::ffi::{self, Py_hash_t};
use crate::panic::PanicException;
use crate::IntoPyPointer;
use crate::{IntoPy, PyObject, Python};
use std::any::Any;
use std::isize;
use std::os::raw::c_int;

Expand Down Expand Up @@ -185,23 +183,3 @@ where
{
value.convert(py)
}

/// Converts the output of std::panic::catch_unwind into a Python function output, either by raising a Python
/// exception or by unwrapping the contained success output.
#[doc(hidden)]
#[inline]
pub fn panic_result_into_callback_output<R>(
py: Python<'_>,
panic_result: Result<PyResult<R>, Box<dyn Any + Send + 'static>>,
) -> R
where
R: PyCallbackOutput,
{
let py_err = match panic_result {
Ok(Ok(value)) => return value,
Ok(Err(py_err)) => py_err,
Err(payload) => PanicException::from_panic_payload(payload),
};
py_err.restore(py);
R::ERR_VALUE
}
111 changes: 48 additions & 63 deletions src/impl_/pyclass.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,29 +313,24 @@ macro_rules! generate_pyclass_getattro_slot {
_slf: *mut $crate::ffi::PyObject,
attr: *mut $crate::ffi::PyObject,
) -> *mut $crate::ffi::PyObject {
use ::std::result::Result::*;
use $crate::impl_::pyclass::*;
let gil = $crate::GILPool::new();
let py = gil.python();
$crate::callback::panic_result_into_callback_output(
py,
::std::panic::catch_unwind(move || -> $crate::PyResult<_> {
let collector = PyClassImplCollector::<$cls>::new();

// Strategy:
// - Try __getattribute__ first. Its default is PyObject_GenericGetAttr.
// - If it returns a result, use it.
// - If it fails with AttributeError, try __getattr__.
// - If it fails otherwise, reraise.
match collector.__getattribute__(py, _slf, attr) {
Ok(obj) => Ok(obj),
Err(e) if e.is_instance_of::<$crate::exceptions::PyAttributeError>(py) => {
collector.__getattr__(py, _slf, attr)
}
Err(e) => Err(e),
$crate::impl_::trampoline::getattrofunc(_slf, attr, |py, _slf, attr| {
use ::std::result::Result::*;
use $crate::impl_::pyclass::*;
let collector = PyClassImplCollector::<$cls>::new();

// Strategy:
// - Try __getattribute__ first. Its default is PyObject_GenericGetAttr.
// - If it returns a result, use it.
// - If it fails with AttributeError, try __getattr__.
// - If it fails otherwise, reraise.
match collector.__getattribute__(py, _slf, attr) {
Ok(obj) => Ok(obj),
Err(e) if e.is_instance_of::<$crate::exceptions::PyAttributeError>(py) => {
collector.__getattr__(py, _slf, attr)
}
}),
)
Err(e) => Err(e),
}
})
}
$crate::ffi::PyType_Slot {
slot: $crate::ffi::Py_tp_getattro,
Expand Down Expand Up @@ -402,21 +397,21 @@ macro_rules! define_pyclass_setattr_slot {
attr: *mut $crate::ffi::PyObject,
value: *mut $crate::ffi::PyObject,
) -> ::std::os::raw::c_int {
use ::std::option::Option::*;
use $crate::callback::IntoPyCallbackOutput;
use $crate::impl_::pyclass::*;
let gil = $crate::GILPool::new();
let py = gil.python();
$crate::callback::panic_result_into_callback_output(
py,
::std::panic::catch_unwind(move || -> $crate::PyResult<_> {
$crate::impl_::trampoline::setattrofunc(
_slf,
attr,
value,
|py, _slf, attr, value| {
use ::std::option::Option::*;
use $crate::callback::IntoPyCallbackOutput;
use $crate::impl_::pyclass::*;
let collector = PyClassImplCollector::<$cls>::new();
if let Some(value) = ::std::ptr::NonNull::new(value) {
collector.$set(py, _slf, attr, value).convert(py)
} else {
collector.$del(py, _slf, attr).convert(py)
}
}),
},
)
}
$crate::ffi::PyType_Slot {
Expand Down Expand Up @@ -517,22 +512,17 @@ macro_rules! define_pyclass_binary_operator_slot {
_slf: *mut $crate::ffi::PyObject,
_other: *mut $crate::ffi::PyObject,
) -> *mut $crate::ffi::PyObject {
let gil = $crate::GILPool::new();
let py = gil.python();
$crate::callback::panic_result_into_callback_output(
py,
::std::panic::catch_unwind(move || -> $crate::PyResult<_> {
use $crate::impl_::pyclass::*;
let collector = PyClassImplCollector::<$cls>::new();
let lhs_result = collector.$lhs(py, _slf, _other)?;
if lhs_result == $crate::ffi::Py_NotImplemented() {
$crate::ffi::Py_DECREF(lhs_result);
collector.$rhs(py, _other, _slf)
} else {
::std::result::Result::Ok(lhs_result)
}
}),
)
$crate::impl_::trampoline::binaryfunc(_slf, _other, |py, _slf, _other| {
use $crate::impl_::pyclass::*;
let collector = PyClassImplCollector::<$cls>::new();
let lhs_result = collector.$lhs(py, _slf, _other)?;
if lhs_result == $crate::ffi::Py_NotImplemented() {
$crate::ffi::Py_DECREF(lhs_result);
collector.$rhs(py, _other, _slf)
} else {
::std::result::Result::Ok(lhs_result)
}
})
}
$crate::ffi::PyType_Slot {
slot: $crate::ffi::$slot,
Expand Down Expand Up @@ -715,22 +705,17 @@ macro_rules! generate_pyclass_pow_slot {
_other: *mut $crate::ffi::PyObject,
_mod: *mut $crate::ffi::PyObject,
) -> *mut $crate::ffi::PyObject {
let gil = $crate::GILPool::new();
let py = gil.python();
$crate::callback::panic_result_into_callback_output(
py,
::std::panic::catch_unwind(move || -> $crate::PyResult<_> {
use $crate::impl_::pyclass::*;
let collector = PyClassImplCollector::<$cls>::new();
let lhs_result = collector.__pow__(py, _slf, _other, _mod)?;
if lhs_result == $crate::ffi::Py_NotImplemented() {
$crate::ffi::Py_DECREF(lhs_result);
collector.__rpow__(py, _other, _slf, _mod)
} else {
::std::result::Result::Ok(lhs_result)
}
}),
)
$crate::impl_::trampoline::ternaryfunc(_slf, _other, _mod, |py, _slf, _other, _mod| {
use $crate::impl_::pyclass::*;
let collector = PyClassImplCollector::<$cls>::new();
let lhs_result = collector.__pow__(py, _slf, _other, _mod)?;
if lhs_result == $crate::ffi::Py_NotImplemented() {
$crate::ffi::Py_DECREF(lhs_result);
collector.__rpow__(py, _other, _slf, _mod)
} else {
::std::result::Result::Ok(lhs_result)
}
})
}
$crate::ffi::PyType_Slot {
slot: $crate::ffi::Py_nb_power,
Expand Down
131 changes: 55 additions & 76 deletions src/impl_/pymodule.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ use std::{
sync::atomic::{self, AtomicBool},
};

use crate::{
callback::panic_result_into_callback_output, exceptions::PyImportError, ffi, types::PyModule,
GILPool, IntoPyPointer, Py, PyObject, PyResult, Python,
};
use crate::{exceptions::PyImportError, ffi, types::PyModule, Py, PyResult, Python};

/// `Sync` wrapper of `ffi::PyModuleDef`.
pub struct ModuleDef {
Expand Down Expand Up @@ -58,7 +55,22 @@ impl ModuleDef {
}
}
/// Builds a module using user given initializer. Used for [`#[pymodule]`][crate::pymodule].
pub fn make_module(&'static self, py: Python<'_>) -> PyResult<PyObject> {
pub fn make_module<'py>(&'static self, py: Python<'py>) -> PyResult<Py<PyModule>> {
#[cfg(all(PyPy, not(Py_3_8)))]
{
const PYPY_GOOD_VERSION: [u8; 3] = [7, 3, 8];
let version = py
.import("sys")?
.getattr("implementation")?
.getattr("version")?;
if version.lt(crate::types::PyTuple::new(py, &PYPY_GOOD_VERSION))? {
let warn = py.import("warnings")?.getattr("warn")?;
warn.call1((
"PyPy 3.7 versions older than 7.3.8 are known to have binary \
compatibility issues which may cause segfaults. Please upgrade.",
))?;
}
}
let module = unsafe {
Py::<PyModule>::from_owned_ptr_or_err(py, ffi::PyModule_Create(self.ffi_def.get()))?
};
Expand All @@ -68,37 +80,7 @@ impl ModuleDef {
));
}
(self.initializer.0)(py, module.as_ref(py))?;
Ok(module.into())
}
/// Implementation of `PyInit_foo` functions generated in [`#[pymodule]`][crate::pymodule]..
///
/// # Safety
/// The Python GIL must be held.
pub unsafe fn module_init(&'static self) -> *mut ffi::PyObject {
let pool = GILPool::new();
let py = pool.python();
let unwind_safe_self = std::panic::AssertUnwindSafe(self);
panic_result_into_callback_output(
py,
std::panic::catch_unwind(move || -> PyResult<_> {
#[cfg(all(PyPy, not(Py_3_8)))]
{
const PYPY_GOOD_VERSION: [u8; 3] = [7, 3, 8];
let version = py
.import("sys")?
.getattr("implementation")?
.getattr("version")?;
if version.lt(crate::types::PyTuple::new(py, &PYPY_GOOD_VERSION))? {
let warn = py.import("warnings")?.getattr("warn")?;
warn.call1((
"PyPy 3.7 versions older than 7.3.8 are known to have binary \
compatibility issues which may cause segfaults. Please upgrade.",
))?;
}
}
Ok(unwind_safe_self.make_module(py)?.into_ptr())
}),
)
Ok(module)
}
}

Expand All @@ -112,46 +94,43 @@ mod tests {

#[test]
fn module_init() {
unsafe {
static MODULE_DEF: ModuleDef = unsafe {
ModuleDef::new(
"test_module\0",
"some doc\0",
ModuleInitializer(|_, m| {
m.add("SOME_CONSTANT", 42)?;
Ok(())
}),
)
};
Python::with_gil(|py| {
let module = MODULE_DEF.module_init();
let pymodule: &PyModule = py.from_owned_ptr(module);
assert_eq!(
pymodule
.getattr("__name__")
.unwrap()
.extract::<&str>()
.unwrap(),
"test_module",
);
assert_eq!(
pymodule
.getattr("__doc__")
.unwrap()
.extract::<&str>()
.unwrap(),
"some doc",
);
assert_eq!(
pymodule
.getattr("SOME_CONSTANT")
.unwrap()
.extract::<u8>()
.unwrap(),
42,
);
})
}
static MODULE_DEF: ModuleDef = unsafe {
ModuleDef::new(
"test_module\0",
"some doc\0",
ModuleInitializer(|_, m| {
m.add("SOME_CONSTANT", 42)?;
Ok(())
}),
)
};
Python::with_gil(|py| {
let module = MODULE_DEF.make_module(py).unwrap().into_ref(py);
assert_eq!(
module
.getattr("__name__")
.unwrap()
.extract::<&str>()
.unwrap(),
"test_module",
);
assert_eq!(
module
.getattr("__doc__")
.unwrap()
.extract::<&str>()
.unwrap(),
"some doc",
);
assert_eq!(
module
.getattr("SOME_CONSTANT")
.unwrap()
.extract::<u8>()
.unwrap(),
42,
);
})
}

#[test]
Expand Down

0 comments on commit 9032f75

Please sign in to comment.