From 2e8010b5df0cc1d69114974d58f1aba151fa5817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20P=C3=BCtz?= Date: Sat, 5 Sep 2020 15:54:03 +0200 Subject: [PATCH 1/7] Add native Function native types. Add bindings for PyCFunction, PyFunction, PyClassMethod and PyStaticMethod. --- CHANGELOG.md | 2 ++ pyo3-derive-backend/src/module.rs | 5 ++--- src/ffi/funcobject.rs | 34 +++++++++++++++++++++++++++++++ src/ffi/mod.rs | 3 +++ src/types/function.rs | 14 +++++++++++++ src/types/mod.rs | 2 ++ src/types/module.rs | 9 ++++---- tests/test_pyfunction.rs | 32 +++++++++++++++++++++++++++++ 8 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 src/ffi/funcobject.rs create mode 100644 src/types/function.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 962ba01e0de..14a95b6afa1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Add macro attribute to `#[pyfn]` and `#[pyfunction]` to pass the module of a Python function to the function body. [#1143](https://github.com/PyO3/pyo3/pull/1143) - Add `add_function()` and `add_submodule()` functions to `PyModule` [#1143](https://github.com/PyO3/pyo3/pull/1143) +- Add native `PyCFunction` and `PyFunction` types, change `add_function` to take a wrapper returning + a `&PyCFunction`instead of `PyObject`. [#1163](https://github.com/PyO3/pyo3/pull/1163) ### Changed - Exception types have been renamed from e.g. `RuntimeError` to `PyRuntimeError`, and are now only accessible by `&T` or `Py` similar to other Python-native types. The old names continue to exist but are deprecated. [#1024](https://github.com/PyO3/pyo3/pull/1024) diff --git a/pyo3-derive-backend/src/module.rs b/pyo3-derive-backend/src/module.rs index a706100ecd2..7b74a9d5dd8 100644 --- a/pyo3-derive-backend/src/module.rs +++ b/pyo3-derive-backend/src/module.rs @@ -209,7 +209,7 @@ pub fn add_fn_to_module( Ok(quote! { fn #function_wrapper_ident<'a>( args: impl Into> - ) -> pyo3::PyResult { + ) -> pyo3::PyResult<&'a pyo3::types::PyCFunction> { let arg = args.into(); let (py, maybe_module) = arg.into_py_and_maybe_module(); #wrapper @@ -231,8 +231,7 @@ pub fn add_fn_to_module( }; let function = unsafe { - pyo3::PyObject::from_owned_ptr( - py, + py.from_owned_ptr::( pyo3::ffi::PyCFunction_NewEx( Box::into_raw(Box::new(_def.as_method_def())), mod_ptr, diff --git a/src/ffi/funcobject.rs b/src/ffi/funcobject.rs new file mode 100644 index 00000000000..10b15345870 --- /dev/null +++ b/src/ffi/funcobject.rs @@ -0,0 +1,34 @@ +use std::os::raw::c_int; + +use crate::ffi::object::{PyObject, PyTypeObject, Py_TYPE}; + +#[cfg_attr(windows, link(name = "pythonXY"))] +extern "C" { + #[cfg_attr(PyPy, link_name = "PyPyFunction_Type")] + pub static mut PyFunction_Type: PyTypeObject; +} + +#[inline] +pub unsafe fn PyFunction_Check(op: *mut PyObject) -> c_int { + (Py_TYPE(op) == &mut PyFunction_Type) as c_int +} + +extern "C" { + pub fn PyFunction_NewWithQualName( + code: *mut PyObject, + globals: *mut PyObject, + qualname: *mut PyObject, + ) -> *mut PyObject; + pub fn PyFunction_New(code: *mut PyObject, globals: *mut PyObject) -> *mut PyObject; + pub fn PyFunction_Code(op: *mut PyObject) -> *mut PyObject; + pub fn PyFunction_GetGlobals(op: *mut PyObject) -> *mut PyObject; + pub fn PyFunction_GetModule(op: *mut PyObject) -> *mut PyObject; + pub fn PyFunction_GetDefaults(op: *mut PyObject) -> *mut PyObject; + pub fn PyFunction_SetDefaults(op: *mut PyObject, defaults: *mut PyObject) -> c_int; + pub fn PyFunction_GetKwDefaults(op: *mut PyObject) -> *mut PyObject; + pub fn PyFunction_SetKwDefaults(op: *mut PyObject, defaults: *mut PyObject) -> c_int; + pub fn PyFunction_GetClosure(op: *mut PyObject) -> *mut PyObject; + pub fn PyFunction_SetClosure(op: *mut PyObject, closure: *mut PyObject) -> c_int; + pub fn PyFunction_GetAnnotations(op: *mut PyObject) -> *mut PyObject; + pub fn PyFunction_SetAnnotations(op: *mut PyObject, annotations: *mut PyObject) -> c_int; +} diff --git a/src/ffi/mod.rs b/src/ffi/mod.rs index 3c5e9e9488e..4015b2cd0bb 100644 --- a/src/ffi/mod.rs +++ b/src/ffi/mod.rs @@ -19,6 +19,7 @@ pub use self::eval::*; pub use self::fileobject::*; pub use self::floatobject::*; pub use self::frameobject::PyFrameObject; +pub use self::funcobject::*; pub use self::genobject::*; pub use self::import::*; pub use self::intrcheck::*; @@ -157,3 +158,5 @@ pub mod frameobject { pub(crate) mod datetime; pub(crate) mod marshal; + +pub(crate) mod funcobject; diff --git a/src/types/function.rs b/src/types/function.rs new file mode 100644 index 00000000000..a28ed7ff24b --- /dev/null +++ b/src/types/function.rs @@ -0,0 +1,14 @@ +use crate::ffi; +use crate::prelude::*; + +/// Represents a builtin Python function object. +#[repr(transparent)] +pub struct PyCFunction(PyAny); + +pyobject_native_var_type!(PyCFunction, ffi::PyCFunction_Type, ffi::PyCFunction_Check); + +/// Represents a Python function object. +#[repr(transparent)] +pub struct PyFunction(PyAny); + +pyobject_native_var_type!(PyFunction, ffi::PyFunction_Type, ffi::PyFunction_Check); diff --git a/src/types/mod.rs b/src/types/mod.rs index 32bbfe8e77e..05887e5c87e 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -13,6 +13,7 @@ pub use self::datetime::{ }; pub use self::dict::{IntoPyDict, PyDict}; pub use self::floatob::PyFloat; +pub use self::function::{PyCFunction, PyFunction}; pub use self::iterator::PyIterator; pub use self::list::PyList; pub use self::module::PyModule; @@ -225,6 +226,7 @@ mod complex; mod datetime; mod dict; mod floatob; +mod function; mod iterator; mod list; mod module; diff --git a/src/types/module.rs b/src/types/module.rs index 0e480010822..8ab4e5deb38 100644 --- a/src/types/module.rs +++ b/src/types/module.rs @@ -9,8 +9,8 @@ use crate::ffi; use crate::instance::PyNativeType; use crate::pyclass::PyClass; use crate::type_object::PyTypeObject; -use crate::types::PyTuple; use crate::types::{PyAny, PyDict, PyList}; +use crate::types::{PyCFunction, PyTuple}; use crate::{AsPyPointer, IntoPy, Py, PyObject, Python}; use std::ffi::{CStr, CString}; use std::os::raw::c_char; @@ -275,12 +275,11 @@ impl PyModule { /// ``` pub fn add_function<'a>( &'a self, - wrapper: &impl Fn(&'a Self) -> PyResult, + wrapper: &impl Fn(&'a Self) -> PyResult<&'a PyCFunction>, ) -> PyResult<()> { - let py = self.py(); let function = wrapper(self)?; - let name = function.getattr(py, "__name__")?; - let name = name.extract(py)?; + let name = function.getattr("__name__")?; + let name = name.extract()?; self.add(name, function) } } diff --git a/tests/test_pyfunction.rs b/tests/test_pyfunction.rs index 0d8500a36fe..f7c3aaf4a8b 100644 --- a/tests/test_pyfunction.rs +++ b/tests/test_pyfunction.rs @@ -1,5 +1,6 @@ use pyo3::buffer::PyBuffer; use pyo3::prelude::*; +use pyo3::types::{PyCFunction, PyFunction}; use pyo3::wrap_pyfunction; mod common; @@ -62,3 +63,34 @@ assert a, array.array("i", [2, 4, 6, 8]) "# ); } + +#[pyfunction] +fn function_with_pyfunction_arg(fun: &PyFunction) -> PyResult<&PyAny> { + fun.call((), None) +} + +#[pyfunction] +fn function_with_pycfunction_arg(fun: &PyCFunction) -> PyResult<&PyAny> { + fun.call((), None) +} + +#[test] +fn test_functions_with_function_args() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let py_func_arg = wrap_pyfunction!(function_with_pyfunction_arg)(py).unwrap(); + let py_cfunc_arg = wrap_pyfunction!(function_with_pycfunction_arg)(py).unwrap(); + let bool_to_string = wrap_pyfunction!(optional_bool)(py).unwrap(); + + pyo3::py_run!( + py, + py_func_arg + py_cfunc_arg + bool_to_string, + r#" + def foo(): return "bar" + assert py_func_arg(foo) == "bar" + assert py_cfunc_arg(bool_to_string) == "Some(true)" + "# + ) +} From be877d133f61b0e0de26c112032115a297907acd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20P=C3=BCtz?= Date: Tue, 8 Sep 2020 14:23:24 +0200 Subject: [PATCH 2/7] Add constructor for PyCFunction. --- pyo3-derive-backend/src/module.rs | 46 +++++------------ src/lib.rs | 10 ++++ src/types/function.rs | 82 ++++++++++++++++++++++++++++++- tests/test_pyfunction.rs | 27 +++++++++- 4 files changed, 129 insertions(+), 36 deletions(-) diff --git a/pyo3-derive-backend/src/module.rs b/pyo3-derive-backend/src/module.rs index 7b74a9d5dd8..dc8e2bc79e5 100644 --- a/pyo3-derive-backend/src/module.rs +++ b/pyo3-derive-backend/src/module.rs @@ -204,49 +204,28 @@ pub fn add_fn_to_module( let python_name = &spec.python_name; - let wrapper = function_c_wrapper(&func.sig.ident, &spec, pyfn_attrs.pass_module); - + let name = &func.sig.ident; + let wrapper_ident = format_ident!("__pyo3_raw_{}", name); + let wrapper = function_c_wrapper(name, &wrapper_ident, &spec, pyfn_attrs.pass_module); Ok(quote! { + #wrapper fn #function_wrapper_ident<'a>( args: impl Into> ) -> pyo3::PyResult<&'a pyo3::types::PyCFunction> { let arg = args.into(); let (py, maybe_module) = arg.into_py_and_maybe_module(); - #wrapper - - let _def = pyo3::class::PyMethodDef { - ml_name: stringify!(#python_name), - ml_meth: pyo3::class::PyMethodType::PyCFunctionWithKeywords(__wrap), - ml_flags: pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS, - ml_doc: #doc, - }; - - let (mod_ptr, name) = if let Some(m) = maybe_module { - let mod_ptr = ::as_ptr(m); - let name = m.name()?; - let name = <&str as pyo3::conversion::IntoPy>::into_py(name, py); - (mod_ptr, ::as_ptr(&name)) - } else { - (std::ptr::null_mut(), std::ptr::null_mut()) - }; - - let function = unsafe { - py.from_owned_ptr::( - pyo3::ffi::PyCFunction_NewEx( - Box::into_raw(Box::new(_def.as_method_def())), - mod_ptr, - name - ) - ) - }; - - Ok(function) + pyo3::types::PyCFunction::new_with_keywords(#wrapper_ident, stringify!(#python_name), #doc, maybe_module, py) } }) } /// Generate static function wrapper (PyCFunction, PyCFunctionWithKeywords) -fn function_c_wrapper(name: &Ident, spec: &method::FnSpec<'_>, pass_module: bool) -> TokenStream { +fn function_c_wrapper( + name: &Ident, + wrapper_ident: &Ident, + spec: &method::FnSpec<'_>, + pass_module: bool, +) -> TokenStream { let names: Vec = get_arg_names(&spec); let cb; let slf_module; @@ -264,9 +243,8 @@ fn function_c_wrapper(name: &Ident, spec: &method::FnSpec<'_>, pass_module: bool slf_module = None; }; let body = pymethod::impl_arg_params(spec, None, cb); - quote! { - unsafe extern "C" fn __wrap( + unsafe extern "C" fn #wrapper_ident( _slf: *mut pyo3::ffi::PyObject, _args: *mut pyo3::ffi::PyObject, _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject diff --git a/src/lib.rs b/src/lib.rs index 4c2313e3a47..12979b53f85 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -220,6 +220,16 @@ macro_rules! wrap_pyfunction { }}; } +/// Returns the function that is called in the C-FFI. +/// +/// Use this together with `#[pyfunction]` and [types::PyCFunction]. +#[macro_export] +macro_rules! raw_pycfunction { + ($function_name: ident) => {{ + pyo3::paste::expr! { [<__pyo3_raw_ $function_name>] } + }}; +} + /// Returns a function that takes a [Python] instance and returns a Python module. /// /// Use this together with `#[pymodule]` and [types::PyModule::add_wrapped]. diff --git a/src/types/function.rs b/src/types/function.rs index a28ed7ff24b..48e1c2d2d62 100644 --- a/src/types/function.rs +++ b/src/types/function.rs @@ -1,5 +1,6 @@ -use crate::ffi; +use crate::exceptions::PyValueError; use crate::prelude::*; +use crate::{class, ffi, AsPyPointer, PyMethodDef, PyMethodType}; /// Represents a builtin Python function object. #[repr(transparent)] @@ -7,6 +8,85 @@ pub struct PyCFunction(PyAny); pyobject_native_var_type!(PyCFunction, ffi::PyCFunction_Type, ffi::PyCFunction_Check); +impl PyCFunction { + /// Create a new built-in function with keywords. + pub fn new_with_keywords<'a>( + fun: ffi::PyCFunctionWithKeywords, + name: &str, + doc: &str, + module: Option<&'a PyModule>, + py: Python<'a>, + ) -> PyResult<&'a PyCFunction> { + let fun = PyMethodType::PyCFunctionWithKeywords(fun); + Self::new_(fun, name, doc, module, py) + } + + /// Create a new built-in function without keywords. + pub fn new<'a>( + fun: ffi::PyCFunction, + name: &str, + doc: &str, + module: Option<&'a PyModule>, + py: Python<'a>, + ) -> PyResult<&'a PyCFunction> { + let fun = PyMethodType::PyCFunction(fun); + Self::new_(fun, name, doc, module, py) + } + + fn new_<'a>( + fun: class::PyMethodType, + name: &str, + doc: &str, + module: Option<&'a PyModule>, + py: Python<'a>, + ) -> PyResult<&'a PyCFunction> { + let name = name.to_string(); + let name: &'static str = Box::leak(name.into_boxed_str()); + // this is ugly but necessary since `PyMethodDef::ml_doc` is &str and not `CStr` + let doc = if doc.ends_with('\0') { + doc.to_string() + } else { + format!("{}\0", doc) + }; + let doc: &'static str = Box::leak(doc.into_boxed_str()); + let def = match &fun { + PyMethodType::PyCFunction(_) => PyMethodDef { + ml_name: name, + ml_meth: fun, + ml_flags: ffi::METH_VARARGS, + ml_doc: doc, + }, + PyMethodType::PyCFunctionWithKeywords(_) => PyMethodDef { + ml_name: name, + ml_meth: fun, + ml_flags: ffi::METH_VARARGS | ffi::METH_KEYWORDS, + ml_doc: doc, + }, + _ => { + return Err(PyValueError::py_err( + "Only PyCFunction and PyCFunctionWithKeywords are valid.", + )) + } + }; + let def = def.as_method_def(); + let (mod_ptr, module_name) = if let Some(m) = module { + let mod_ptr = m.as_ptr(); + let name = m.name()?.into_py(py); + (mod_ptr, name.as_ptr()) + } else { + (std::ptr::null_mut(), std::ptr::null_mut()) + }; + + unsafe { + py.from_owned_ptr_or_err::(ffi::PyCFunction_NewEx( + Box::into_raw(Box::new(def)), + mod_ptr, + module_name, + )) + } + } +} + /// Represents a Python function object. #[repr(transparent)] pub struct PyFunction(PyAny); diff --git a/tests/test_pyfunction.rs b/tests/test_pyfunction.rs index f7c3aaf4a8b..6de5637ae3a 100644 --- a/tests/test_pyfunction.rs +++ b/tests/test_pyfunction.rs @@ -1,7 +1,7 @@ use pyo3::buffer::PyBuffer; use pyo3::prelude::*; use pyo3::types::{PyCFunction, PyFunction}; -use pyo3::wrap_pyfunction; +use pyo3::{raw_pycfunction, wrap_pyfunction}; mod common; @@ -94,3 +94,28 @@ fn test_functions_with_function_args() { "# ) } + +#[test] +fn test_raw_function() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let raw_func = raw_pycfunction!(optional_bool); + let fun = PyCFunction::new_with_keywords(raw_func, "fun", "", None, py).unwrap(); + let res = fun.call((), None).unwrap().extract::<&str>().unwrap(); + assert_eq!(res, "Some(true)"); + let res = fun.call((false,), None).unwrap().extract::<&str>().unwrap(); + assert_eq!(res, "Some(false)"); + let no_module = fun.getattr("__module__").unwrap().is_none(); + assert!(no_module); + + let module = PyModule::new(py, "cool_module").unwrap(); + module.add_function(&|_| Ok(fun)).unwrap(); + let res = module + .getattr("fun") + .unwrap() + .call((), None) + .unwrap() + .extract::<&str>() + .unwrap(); + assert_eq!(res, "Some(true)"); +} From 22881a3c2ffbd1adb65b95159e3de0ea36d73d77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20P=C3=BCtz?= Date: Wed, 9 Sep 2020 11:53:24 +0200 Subject: [PATCH 3/7] Change add_function, ensure static docstrings. Change add_function to take `&PyCFunction` instead of a wrapper fn and ensure that dostrings of functions are `&'static str`. --- README.md | 2 +- examples/rustapi_module/src/datetime.rs | 28 ++++++------ examples/rustapi_module/src/othermod.rs | 2 +- examples/word-count/src/lib.rs | 4 +- guide/src/function.md | 6 +-- guide/src/module.md | 2 +- guide/src/trait_bounds.md | 2 +- pyo3-derive-backend/src/module.rs | 8 ++-- src/derive_utils.rs | 21 +++++---- src/lib.rs | 6 ++- src/python.rs | 2 +- src/types/function.rs | 57 +++++++++++-------------- src/types/module.rs | 15 +++---- tests/test_module.rs | 32 +++++++------- tests/test_pyfunction.rs | 4 +- 15 files changed, 92 insertions(+), 99 deletions(-) diff --git a/README.md b/README.md index ff041bf60b7..2e57a358de1 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ fn sum_as_string(a: usize, b: usize) -> PyResult { /// A Python module implemented in Rust. #[pymodule] fn string_sum(py: Python, m: &PyModule) -> PyResult<()> { - m.add_function(wrap_pyfunction!(sum_as_string))?; + m.add_function(wrap_pyfunction!(sum_as_string, m)?)?; Ok(()) } diff --git a/examples/rustapi_module/src/datetime.rs b/examples/rustapi_module/src/datetime.rs index 3ccb7c697f6..062dc958b97 100644 --- a/examples/rustapi_module/src/datetime.rs +++ b/examples/rustapi_module/src/datetime.rs @@ -215,29 +215,29 @@ impl TzClass { #[pymodule] fn datetime(_py: Python<'_>, m: &PyModule) -> PyResult<()> { - m.add_function(wrap_pyfunction!(make_date))?; - m.add_function(wrap_pyfunction!(get_date_tuple))?; - m.add_function(wrap_pyfunction!(date_from_timestamp))?; - m.add_function(wrap_pyfunction!(make_time))?; - m.add_function(wrap_pyfunction!(get_time_tuple))?; - m.add_function(wrap_pyfunction!(make_delta))?; - m.add_function(wrap_pyfunction!(get_delta_tuple))?; - m.add_function(wrap_pyfunction!(make_datetime))?; - m.add_function(wrap_pyfunction!(get_datetime_tuple))?; - m.add_function(wrap_pyfunction!(datetime_from_timestamp))?; + m.add_function(wrap_pyfunction!(make_date, m)?)?; + m.add_function(wrap_pyfunction!(get_date_tuple, m)?)?; + m.add_function(wrap_pyfunction!(date_from_timestamp, m)?)?; + m.add_function(wrap_pyfunction!(make_time, m)?)?; + m.add_function(wrap_pyfunction!(get_time_tuple, m)?)?; + m.add_function(wrap_pyfunction!(make_delta, m)?)?; + m.add_function(wrap_pyfunction!(get_delta_tuple, m)?)?; + m.add_function(wrap_pyfunction!(make_datetime, m)?)?; + m.add_function(wrap_pyfunction!(get_datetime_tuple, m)?)?; + m.add_function(wrap_pyfunction!(datetime_from_timestamp, m)?)?; // Python 3.6+ functions #[cfg(Py_3_6)] { - m.add_function(wrap_pyfunction!(time_with_fold))?; + m.add_function(wrap_pyfunction!(time_with_fold, m)?)?; #[cfg(not(PyPy))] { - m.add_function(wrap_pyfunction!(get_time_tuple_fold))?; - m.add_function(wrap_pyfunction!(get_datetime_tuple_fold))?; + m.add_function(wrap_pyfunction!(get_time_tuple_fold, m)?)?; + m.add_function(wrap_pyfunction!(get_datetime_tuple_fold, m)?)?; } } - m.add_function(wrap_pyfunction!(issue_219))?; + m.add_function(wrap_pyfunction!(issue_219, m)?)?; m.add_class::()?; Ok(()) diff --git a/examples/rustapi_module/src/othermod.rs b/examples/rustapi_module/src/othermod.rs index b9955806186..5c149d29ed8 100644 --- a/examples/rustapi_module/src/othermod.rs +++ b/examples/rustapi_module/src/othermod.rs @@ -31,7 +31,7 @@ fn double(x: i32) -> i32 { #[pymodule] fn othermod(_py: Python<'_>, m: &PyModule) -> PyResult<()> { - m.add_function(wrap_pyfunction!(double))?; + m.add_function(wrap_pyfunction!(double, m)?)?; m.add_class::()?; diff --git a/examples/word-count/src/lib.rs b/examples/word-count/src/lib.rs index 50a0078026b..78213102129 100644 --- a/examples/word-count/src/lib.rs +++ b/examples/word-count/src/lib.rs @@ -56,8 +56,8 @@ fn count_line(line: &str, needle: &str) -> usize { #[pymodule] fn word_count(_py: Python<'_>, m: &PyModule) -> PyResult<()> { m.add_wrapped(wrap_pyfunction!(search))?; - m.add_function(wrap_pyfunction!(search_sequential))?; - m.add_function(wrap_pyfunction!(search_sequential_allow_threads))?; + m.add_function(wrap_pyfunction!(search_sequential, m)?)?; + m.add_function(wrap_pyfunction!(search_sequential_allow_threads, m)?)?; Ok(()) } diff --git a/guide/src/function.md b/guide/src/function.md index b33221c9d41..0e0277f3a67 100644 --- a/guide/src/function.md +++ b/guide/src/function.md @@ -36,7 +36,7 @@ fn double(x: usize) -> usize { #[pymodule] fn module_with_functions(py: Python, m: &PyModule) -> PyResult<()> { - m.add_function(wrap_pyfunction!(double)).unwrap(); + m.add_function(wrap_pyfunction!(double, m)?).unwrap(); Ok(()) } @@ -65,7 +65,7 @@ fn num_kwds(kwds: Option<&PyDict>) -> usize { #[pymodule] fn module_with_functions(py: Python, m: &PyModule) -> PyResult<()> { - m.add_function(wrap_pyfunction!(num_kwds)).unwrap(); + m.add_function(wrap_pyfunction!(num_kwds, m)?).unwrap(); Ok(()) } @@ -206,7 +206,7 @@ fn pyfunction_with_module(module: &PyModule) -> PyResult<&str> { #[pymodule] fn module_with_fn(py: Python, m: &PyModule) -> PyResult<()> { - m.add_function(wrap_pyfunction!(pyfunction_with_module)) + m.add_function(wrap_pyfunction!(pyfunction_with_module, m)?) } # fn main() {} diff --git a/guide/src/module.md b/guide/src/module.md index 6b1d4581eec..aabba50b392 100644 --- a/guide/src/module.md +++ b/guide/src/module.md @@ -73,7 +73,7 @@ fn subfunction() -> String { } fn init_submodule(module: &PyModule) -> PyResult<()> { - module.add_function(wrap_pyfunction!(subfunction))?; + module.add_function(wrap_pyfunction!(subfunction, module)?)?; Ok(()) } diff --git a/guide/src/trait_bounds.md b/guide/src/trait_bounds.md index 65e173cd40d..f845b5c898c 100644 --- a/guide/src/trait_bounds.md +++ b/guide/src/trait_bounds.md @@ -488,7 +488,7 @@ pub struct UserModel { #[pymodule] fn trait_exposure(_py: Python, m: &PyModule) -> PyResult<()> { m.add_class::()?; - m.add_function(wrap_pyfunction!(solve_wrapper))?; + m.add_function(wrap_pyfunction!(solve_wrapper, m)?)?; Ok(()) } diff --git a/pyo3-derive-backend/src/module.rs b/pyo3-derive-backend/src/module.rs index dc8e2bc79e5..657d1134a8c 100644 --- a/pyo3-derive-backend/src/module.rs +++ b/pyo3-derive-backend/src/module.rs @@ -44,7 +44,7 @@ pub fn process_functions_in_module(func: &mut syn::ItemFn) -> syn::Result<()> { let item: syn::ItemFn = syn::parse_quote! { fn block_wrapper() { #function_to_python - #module_name.add_function(&#function_wrapper_ident)?; + #module_name.add_function(#function_wrapper_ident(#module_name)?)?; } }; stmts.extend(item.block.stmts.into_iter()); @@ -210,11 +210,9 @@ pub fn add_fn_to_module( Ok(quote! { #wrapper fn #function_wrapper_ident<'a>( - args: impl Into> + args: impl Into> ) -> pyo3::PyResult<&'a pyo3::types::PyCFunction> { - let arg = args.into(); - let (py, maybe_module) = arg.into_py_and_maybe_module(); - pyo3::types::PyCFunction::new_with_keywords(#wrapper_ident, stringify!(#python_name), #doc, maybe_module, py) + pyo3::types::PyCFunction::new_with_keywords(#wrapper_ident, stringify!(#python_name), #doc, args.into()) } }) } diff --git a/src/derive_utils.rs b/src/derive_utils.rs index cef90fd1248..819599e991c 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -209,17 +209,16 @@ where } /// Enum to abstract over the arguments of Python function wrappers. -#[doc(hidden)] -pub enum WrapPyFunctionArguments<'a> { +pub enum PyFunctionArguments<'a> { Python(Python<'a>), PyModule(&'a PyModule), } -impl<'a> WrapPyFunctionArguments<'a> { +impl<'a> PyFunctionArguments<'a> { pub fn into_py_and_maybe_module(self) -> (Python<'a>, Option<&'a PyModule>) { match self { - WrapPyFunctionArguments::Python(py) => (py, None), - WrapPyFunctionArguments::PyModule(module) => { + PyFunctionArguments::Python(py) => (py, None), + PyFunctionArguments::PyModule(module) => { let py = module.py(); (py, Some(module)) } @@ -227,14 +226,14 @@ impl<'a> WrapPyFunctionArguments<'a> { } } -impl<'a> From> for WrapPyFunctionArguments<'a> { - fn from(py: Python<'a>) -> WrapPyFunctionArguments<'a> { - WrapPyFunctionArguments::Python(py) +impl<'a> From> for PyFunctionArguments<'a> { + fn from(py: Python<'a>) -> PyFunctionArguments<'a> { + PyFunctionArguments::Python(py) } } -impl<'a> From<&'a PyModule> for WrapPyFunctionArguments<'a> { - fn from(module: &'a PyModule) -> WrapPyFunctionArguments<'a> { - WrapPyFunctionArguments::PyModule(module) +impl<'a> From<&'a PyModule> for PyFunctionArguments<'a> { + fn from(module: &'a PyModule) -> PyFunctionArguments<'a> { + PyFunctionArguments::PyModule(module) } } diff --git a/src/lib.rs b/src/lib.rs index 12979b53f85..2935c390b0f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -71,7 +71,7 @@ //! #[pymodule] //! /// A Python module implemented in Rust. //! fn string_sum(py: Python, m: &PyModule) -> PyResult<()> { -//! m.add_function(wrap_pyfunction!(sum_as_string))?; +//! m.add_function(wrap_pyfunction!(sum_as_string, m)?)?; //! //! Ok(()) //! } @@ -218,6 +218,10 @@ macro_rules! wrap_pyfunction { ($function_name: ident) => {{ &pyo3::paste::expr! { [<__pyo3_get_function_ $function_name>] } }}; + + ($function_name: ident, $arg: expr) => { + pyo3::wrap_pyfunction!($function_name)(pyo3::derive_utils::PyFunctionArguments::from($arg)) + }; } /// Returns the function that is called in the C-FFI. diff --git a/src/python.rs b/src/python.rs index db4abfe6b1a..4afec259f78 100644 --- a/src/python.rs +++ b/src/python.rs @@ -134,7 +134,7 @@ impl<'p> Python<'p> { /// let gil = Python::acquire_gil(); /// let py = gil.python(); /// let m = PyModule::new(py, "pcount").unwrap(); - /// m.add_function(wrap_pyfunction!(parallel_count)).unwrap(); + /// m.add_function(wrap_pyfunction!(parallel_count, m).unwrap()).unwrap(); /// let locals = [("pcount", m)].into_py_dict(py); /// py.run(r#" /// s = ["Flow", "my", "tears", "the", "Policeman", "Said"] diff --git a/src/types/function.rs b/src/types/function.rs index 48e1c2d2d62..73aa04b7426 100644 --- a/src/types/function.rs +++ b/src/types/function.rs @@ -1,6 +1,9 @@ +use std::ffi::{CStr, CString}; + +use crate::derive_utils::PyFunctionArguments; use crate::exceptions::PyValueError; use crate::prelude::*; -use crate::{class, ffi, AsPyPointer, PyMethodDef, PyMethodType}; +use crate::{class, ffi, AsPyPointer, PyMethodType}; /// Represents a builtin Python function object. #[repr(transparent)] @@ -13,54 +16,47 @@ impl PyCFunction { pub fn new_with_keywords<'a>( fun: ffi::PyCFunctionWithKeywords, name: &str, - doc: &str, - module: Option<&'a PyModule>, - py: Python<'a>, + doc: &'static str, + py_or_module: PyFunctionArguments<'a>, ) -> PyResult<&'a PyCFunction> { let fun = PyMethodType::PyCFunctionWithKeywords(fun); - Self::new_(fun, name, doc, module, py) + Self::new_(fun, name, doc, py_or_module) } /// Create a new built-in function without keywords. pub fn new<'a>( fun: ffi::PyCFunction, name: &str, - doc: &str, - module: Option<&'a PyModule>, - py: Python<'a>, + doc: &'static str, + py_or_module: PyFunctionArguments<'a>, ) -> PyResult<&'a PyCFunction> { let fun = PyMethodType::PyCFunction(fun); - Self::new_(fun, name, doc, module, py) + Self::new_(fun, name, doc, py_or_module) } fn new_<'a>( fun: class::PyMethodType, name: &str, - doc: &str, - module: Option<&'a PyModule>, - py: Python<'a>, + doc: &'static str, + py_or_module: PyFunctionArguments<'a>, ) -> PyResult<&'a PyCFunction> { - let name = name.to_string(); - let name: &'static str = Box::leak(name.into_boxed_str()); - // this is ugly but necessary since `PyMethodDef::ml_doc` is &str and not `CStr` - let doc = if doc.ends_with('\0') { - doc.to_string() - } else { - format!("{}\0", doc) - }; - let doc: &'static str = Box::leak(doc.into_boxed_str()); - let def = match &fun { - PyMethodType::PyCFunction(_) => PyMethodDef { - ml_name: name, - ml_meth: fun, + let (py, module) = py_or_module.into_py_and_maybe_module(); + let doc: &'static CStr = CStr::from_bytes_with_nul(doc.as_bytes()) + .map_err(|_| PyValueError::py_err("docstring must end with NULL byte."))?; + let name = CString::new(name.as_bytes()) + .map_err(|_| PyValueError::py_err("Function name cannot contain contain NULL byte."))?; + let def = match fun { + PyMethodType::PyCFunction(fun) => ffi::PyMethodDef { + ml_name: name.into_raw() as _, + ml_meth: Some(fun), ml_flags: ffi::METH_VARARGS, - ml_doc: doc, + ml_doc: doc.as_ptr() as _, }, - PyMethodType::PyCFunctionWithKeywords(_) => PyMethodDef { - ml_name: name, - ml_meth: fun, + PyMethodType::PyCFunctionWithKeywords(fun) => ffi::PyMethodDef { + ml_name: name.into_raw() as _, + ml_meth: Some(unsafe { std::mem::transmute(fun) }), ml_flags: ffi::METH_VARARGS | ffi::METH_KEYWORDS, - ml_doc: doc, + ml_doc: doc.as_ptr() as _, }, _ => { return Err(PyValueError::py_err( @@ -68,7 +64,6 @@ impl PyCFunction { )) } }; - let def = def.as_method_def(); let (mod_ptr, module_name) = if let Some(m) = module { let mod_ptr = m.as_ptr(); let name = m.name()?.into_py(py); diff --git a/src/types/module.rs b/src/types/module.rs index 8ab4e5deb38..57f91ee787f 100644 --- a/src/types/module.rs +++ b/src/types/module.rs @@ -256,7 +256,7 @@ impl PyModule { /// } /// #[pymodule] /// fn double_mod(_py: Python, module: &PyModule) -> PyResult<()> { - /// module.add_function(pyo3::wrap_pyfunction!(double)) + /// module.add_function(pyo3::wrap_pyfunction!(double, module)?) /// } /// ``` /// @@ -270,16 +270,11 @@ impl PyModule { /// } /// #[pymodule] /// fn double_mod(_py: Python, module: &PyModule) -> PyResult<()> { - /// module.add("also_double", pyo3::wrap_pyfunction!(double)(module)?) + /// module.add("also_double", pyo3::wrap_pyfunction!(double, module)?) /// } /// ``` - pub fn add_function<'a>( - &'a self, - wrapper: &impl Fn(&'a Self) -> PyResult<&'a PyCFunction>, - ) -> PyResult<()> { - let function = wrapper(self)?; - let name = function.getattr("__name__")?; - let name = name.extract()?; - self.add(name, function) + pub fn add_function<'a>(&'a self, fun: &'a PyCFunction) -> PyResult<()> { + let name = fun.getattr("__name__")?.extract()?; + self.add(name, fun) } } diff --git a/tests/test_module.rs b/tests/test_module.rs index 7c278bdcd5e..5ed75bf4b41 100644 --- a/tests/test_module.rs +++ b/tests/test_module.rs @@ -65,8 +65,8 @@ fn module_with_functions(_py: Python, m: &PyModule) -> PyResult<()> { m.add("foo", "bar").unwrap(); - m.add_function(wrap_pyfunction!(double)).unwrap(); - m.add("also_double", wrap_pyfunction!(double)(m)?).unwrap(); + m.add_function(wrap_pyfunction!(double, m)?).unwrap(); + m.add("also_double", wrap_pyfunction!(double, m)?).unwrap(); Ok(()) } @@ -163,7 +163,7 @@ fn r#move() -> usize { fn raw_ident_module(_py: Python, module: &PyModule) -> PyResult<()> { use pyo3::wrap_pyfunction; - module.add_function(wrap_pyfunction!(r#move)) + module.add_function(wrap_pyfunction!(r#move, module)?) } #[test] @@ -188,7 +188,7 @@ fn custom_named_fn() -> usize { fn foobar_module(_py: Python, m: &PyModule) -> PyResult<()> { use pyo3::wrap_pyfunction; - m.add_function(wrap_pyfunction!(custom_named_fn))?; + m.add_function(wrap_pyfunction!(custom_named_fn, m)?)?; m.dict().set_item("yay", "me")?; Ok(()) } @@ -221,7 +221,7 @@ fn subfunction() -> String { fn submodule(module: &PyModule) -> PyResult<()> { use pyo3::wrap_pyfunction; - module.add_function(wrap_pyfunction!(subfunction))?; + module.add_function(wrap_pyfunction!(subfunction, module)?)?; Ok(()) } @@ -229,7 +229,7 @@ fn submodule(module: &PyModule) -> PyResult<()> { fn submodule_with_init_fn(_py: Python, module: &PyModule) -> PyResult<()> { use pyo3::wrap_pyfunction; - module.add_function(wrap_pyfunction!(subfunction))?; + module.add_function(wrap_pyfunction!(subfunction, module)?)?; Ok(()) } @@ -242,7 +242,7 @@ fn superfunction() -> String { fn supermodule(py: Python, module: &PyModule) -> PyResult<()> { use pyo3::wrap_pyfunction; - module.add_function(wrap_pyfunction!(superfunction))?; + module.add_function(wrap_pyfunction!(superfunction, module)?)?; let module_to_add = PyModule::new(py, "submodule")?; submodule(module_to_add)?; module.add_submodule(module_to_add)?; @@ -291,7 +291,7 @@ fn vararg_module(_py: Python, m: &PyModule) -> PyResult<()> { ext_vararg_fn(py, a, vararg) } - m.add_function(pyo3::wrap_pyfunction!(ext_vararg_fn)) + m.add_function(pyo3::wrap_pyfunction!(ext_vararg_fn, m)?) .unwrap(); Ok(()) } @@ -368,15 +368,17 @@ fn pyfunction_with_module_and_args_kwargs<'a>( #[pymodule] fn module_with_functions_with_module(_py: Python, m: &PyModule) -> PyResult<()> { - m.add_function(pyo3::wrap_pyfunction!(pyfunction_with_module))?; - m.add_function(pyo3::wrap_pyfunction!(pyfunction_with_module_and_py))?; - m.add_function(pyo3::wrap_pyfunction!(pyfunction_with_module_and_arg))?; + m.add_function(pyo3::wrap_pyfunction!(pyfunction_with_module, m)?)?; + m.add_function(pyo3::wrap_pyfunction!(pyfunction_with_module_and_py, m)?)?; + m.add_function(pyo3::wrap_pyfunction!(pyfunction_with_module_and_arg, m)?)?; m.add_function(pyo3::wrap_pyfunction!( - pyfunction_with_module_and_default_arg - ))?; + pyfunction_with_module_and_default_arg, + m + )?)?; m.add_function(pyo3::wrap_pyfunction!( - pyfunction_with_module_and_args_kwargs - )) + pyfunction_with_module_and_args_kwargs, + m + )?) } #[test] diff --git a/tests/test_pyfunction.rs b/tests/test_pyfunction.rs index 6de5637ae3a..89b4dbd01e3 100644 --- a/tests/test_pyfunction.rs +++ b/tests/test_pyfunction.rs @@ -100,7 +100,7 @@ fn test_raw_function() { let gil = Python::acquire_gil(); let py = gil.python(); let raw_func = raw_pycfunction!(optional_bool); - let fun = PyCFunction::new_with_keywords(raw_func, "fun", "", None, py).unwrap(); + let fun = PyCFunction::new_with_keywords(raw_func, "fun", "\0", py.into()).unwrap(); let res = fun.call((), None).unwrap().extract::<&str>().unwrap(); assert_eq!(res, "Some(true)"); let res = fun.call((false,), None).unwrap().extract::<&str>().unwrap(); @@ -109,7 +109,7 @@ fn test_raw_function() { assert!(no_module); let module = PyModule::new(py, "cool_module").unwrap(); - module.add_function(&|_| Ok(fun)).unwrap(); + module.add_function(fun).unwrap(); let res = module .getattr("fun") .unwrap() From b9e95dc7c92b09d07e3a1f4a54e65d6753eda548 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Tue, 25 Aug 2020 20:33:36 +0100 Subject: [PATCH 4/7] Implement std::error::Error for PyErr --- CHANGELOG.md | 16 +- guide/src/exception.md | 46 +- guide/src/migration.md | 86 ++- pyo3-derive-backend/src/from_pyobject.rs | 4 +- src/buffer.rs | 12 +- src/callback.rs | 8 +- src/class/basic.rs | 4 +- src/class/iter.rs | 2 +- src/class/macros.rs | 12 +- src/class/mapping.rs | 1 - src/class/pyasync.rs | 2 +- src/class/sequence.rs | 2 +- src/derive_utils.rs | 2 +- src/err.rs | 673 ----------------------- src/err/err_state.rs | 69 +++ src/err/impls.rs | 114 ++++ src/err/mod.rs | 576 +++++++++++++++++++ src/exceptions.rs | 120 ++-- src/gil.rs | 9 +- src/lib.rs | 8 +- src/pycell.rs | 4 +- src/pyclass.rs | 2 +- src/python.rs | 2 +- src/types/any.rs | 2 +- src/types/bytearray.rs | 2 +- src/types/bytes.rs | 2 +- src/types/num.rs | 4 +- src/types/sequence.rs | 2 +- src/types/tuple.rs | 2 +- tests/test_buffer_protocol.rs | 4 +- tests/test_dunder.rs | 6 +- tests/test_exceptions.rs | 2 +- tests/test_frompyobject.rs | 21 +- tests/test_mapping.rs | 4 +- tests/test_sequence.rs | 8 +- tests/test_various.rs | 2 +- 36 files changed, 992 insertions(+), 843 deletions(-) delete mode 100644 src/err.rs create mode 100644 src/err/err_state.rs create mode 100644 src/err/impls.rs create mode 100644 src/err/mod.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index e3886d0e1da..edfd40c8553 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog -All notable changes to this project will be documented in this file. +All notable changes to this project will be documented in this file. For help with updating to new +PyO3 versions, please see the [migration guide](https://pyo3.rs/master/migration.html). The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). @@ -17,7 +18,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Add `#[derive(FromPyObject)]` macro for enums and structs. [#1065](https://github.com/PyO3/pyo3/pull/1065) ### Changed -- Exception types have been renamed from e.g. `RuntimeError` to `PyRuntimeError`, and are now only accessible by `&T` or `Py` similar to other Python-native types. The old names continue to exist but are deprecated. [#1024](https://github.com/PyO3/pyo3/pull/1024) +- Exception types have been renamed from e.g. `RuntimeError` to `PyRuntimeError`, and are now accessible by `&T` or `Py` similar to other Python-native types. The old names continue to exist but are deprecated. [#1024](https://github.com/PyO3/pyo3/pull/1024) [#1115](https://github.com/PyO3/pyo3/pull/1115) + - Rename `PyException::py_err()` to `PyException::new_err()`. + - Rename `PyUnicodeDecodeErr::new_err()` to `PyUnicodeDecodeErr::new()`. + - Remove `PyStopIteration::stop_iteration()`. - Correct FFI definitions `Py_SetProgramName` and `Py_SetPythonHome` to take `*const` argument instead of `*mut`. [#1021](https://github.com/PyO3/pyo3/pull/1021) - Rename `PyString::to_string` to `to_str`, change return type `Cow` to `&str`. [#1023](https://github.com/PyO3/pyo3/pull/1023) - Correct FFI definition `_PyLong_AsByteArray` `*mut c_uchar` argument instead of `*const c_uchar`. [#1029](https://github.com/PyO3/pyo3/pull/1029) @@ -27,7 +31,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Change `PyIterator::from_object` to return `PyResult` instead of `Result`. [#1051](https://github.com/PyO3/pyo3/pull/1051) - `IntoPy` is no longer implied by `FromPy`. [#1063](https://github.com/PyO3/pyo3/pull/1063) - `PyObject` is now just a type alias for `Py`. [#1063](https://github.com/PyO3/pyo3/pull/1063) -- Implement `Send + Sync` for `PyErr`. `PyErr::new`, `PyErr::from_type`, `PyException::py_err` and `PyException::into` have had these bounds added to their arguments. [#1067](https://github.com/PyO3/pyo3/pull/1067) +- Rework PyErr to be compatible with the `std::error::Error` trait: [#1067](https://github.com/PyO3/pyo3/pull/1067) [#1115](https://github.com/PyO3/pyo3/pull/1115) + - Implement `Display`, `Error`, `Send` and `Sync` for `PyErr` and `PyErrArguments`. + - Add `PyErr::instance()` which returns `&PyBaseException`. + - `PyErr`'s fields are now an implementation detail. The equivalent values can be accessed with `PyErr::ptype()`, `PyErr::pvalue()` and `PyErr::ptraceback()`. + - Change `PyErr::print()` and `PyErr::print_and_set_sys_last_vars()` to take `&self` instead of `self`. + - Remove `PyErrValue`, `PyErr::from_value`, `PyErr::into_normalized()`, and `PyErr::normalize()`. + - Remove `PyException::into()` and `Into>` for `PyErr` and `PyException`. - Change `#[pyproto]` to return NotImplemented for operators for which Python can try a reversed operation. #[1072](https://github.com/PyO3/pyo3/pull/1072) - `PyModule::add` now uses `IntoPy` instead of `ToPyObject`. #[1124](https://github.com/PyO3/pyo3/pull/1124) diff --git a/guide/src/exception.md b/guide/src/exception.md index 3a9144f1dee..62ab6422484 100644 --- a/guide/src/exception.md +++ b/guide/src/exception.md @@ -61,7 +61,7 @@ use pyo3::exceptions::PyTypeError; fn main() { let gil = Python::acquire_gil(); let py = gil.python(); - PyErr::new::("Error").restore(py); + PyTypeError::new_err("Error").restore(py); assert!(PyErr::occurred(py)); drop(PyErr::fetch(py)); } @@ -76,7 +76,7 @@ If you already have a Python exception instance, you can simply call [`PyErr::fr PyErr::from_instance(py, err).restore(py); ``` -If a Rust type exists for the exception, then it is possible to use the `py_err` method. +If a Rust type exists for the exception, then it is possible to use the `new_err` method. For example, each standard exception defined in the `pyo3::exceptions` module has a corresponding Rust type, exceptions defined by [`create_exception!`] and [`import_exception!`] macro have Rust types as well. @@ -87,7 +87,7 @@ have Rust types as well. # fn check_for_error() -> bool {false} fn my_func(arg: PyObject) -> PyResult<()> { if check_for_error() { - Err(PyValueError::py_err("argument is wrong")) + Err(PyValueError::new_err("argument is wrong")) } else { Ok(()) } @@ -115,41 +115,32 @@ fn main() { [`Python::is_instance`] calls the underlying [`PyType::is_instance`](https://docs.rs/pyo3/latest/pyo3/types/struct.PyType.html#method.is_instance) method to do the actual work. -To check the type of an exception, you can simply do: +To check the type of an exception, you can similarly do: ```rust # use pyo3::exceptions::PyTypeError; # use pyo3::prelude::*; -# fn main() { # let gil = Python::acquire_gil(); # let py = gil.python(); -# let err = PyTypeError::py_err(()); +# let err = PyTypeError::new_err(()); err.is_instance::(py); -# } ``` ## Handling Rust errors -The vast majority of operations in this library will return [`PyResult`](https://docs.rs/pyo3/latest/pyo3/prelude/type.PyResult.html), +The vast majority of operations in this library will return +[`PyResult`](https://docs.rs/pyo3/latest/pyo3/prelude/type.PyResult.html), which is an alias for the type `Result`. -A [`PyErr`] represents a Python exception. -Errors within the PyO3 library are also exposed as Python exceptions. - -The PyO3 library handles Python exceptions in two stages. During the first stage, a [`PyErr`] instance is -created. At this stage, holding Python's GIL is not required. During the second stage, an actual Python -exception instance is created and set active in the Python interpreter. +A [`PyErr`] represents a Python exception. Errors within the PyO3 library are also exposed as +Python exceptions. -In simple cases, for custom errors adding an implementation of `std::convert::From` trait -for this custom error is enough. `PyErr::new` accepts an argument in the form -of `ToPyObject + 'static`. If the `'static` constraint can not be satisfied or -more complex arguments are required, the -[`PyErrArguments`](https://docs.rs/pyo3/latest/pyo3/trait.PyErrArguments.html) -trait can be implemented. In that case, actual exception argument creation is delayed -until a `Python` object is available. +If your code has a custom error type e.g. `MyError`, adding an implementation of +`std::convert::From for PyErr` is usually enough. PyO3 will then automatically convert +your error to a Python exception when needed. ```rust -# use pyo3::{PyErr, PyResult}; +# use pyo3::prelude::*; # use pyo3::exceptions::PyOSError; # use std::error::Error; # use std::fmt; @@ -170,11 +161,12 @@ until a `Python` object is available. # } impl std::convert::From for PyErr { fn from(err: CustomIOError) -> PyErr { - PyOSError::py_err(err.to_string()) + PyOSError::new_err(err.to_string()) } } -fn connect(s: String) -> PyResult { +#[pyfunction] +fn connect(s: String) -> Result { bind("127.0.0.1:80")?; Ok(true) } @@ -195,6 +187,10 @@ fn parse_int(s: String) -> PyResult { The code snippet above will raise a `ValueError` in Python if `String::parse()` returns an error. +If lazy construction of the Python exception instance is desired, the +[`PyErrArguments`](https://docs.rs/pyo3/latest/pyo3/trait.PyErrArguments.html) +trait can be implemented. In that case, actual exception argument creation is delayed +until the `PyErr` is needed. ## Using exceptions defined in Python code @@ -213,7 +209,7 @@ fn tell(file: &PyAny) -> PyResult { use pyo3::exceptions::*; match file.call_method0("tell") { - Err(_) => Err(io::UnsupportedOperation::py_err("not supported: tell")), + Err(_) => Err(io::UnsupportedOperation::new_err("not supported: tell")), Ok(x) => x.extract::(), } } diff --git a/guide/src/migration.md b/guide/src/migration.md index 4aefab997c3..82ac707b1f0 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -5,13 +5,88 @@ For a detailed list of all changes, see [CHANGELOG.md](https://github.com/PyO3/p ## from 0.11.* to 0.12 +### `PyErr` has been reworked + +In PyO3 `0.12` the `PyErr` type has been re-implemented to be significantly more compatible with +the standard Rust error handling ecosystem. Specificially `PyErr` now implements +`Error + Send + Sync`, which are the standard traits used for error types. + +While this has necessitated the removal of a number of APIs, the resulting `PyErr` type should now +be much more easier to work with. The following sections list the changes in detail and how to +migrate to the new APIs. + +#### `PyErr::new` and `PyErr::from_type` now require `Send + Sync` for their argument + +For most uses no change will be needed. If you are trying to construct `PyErr` from a value that is +not `Send + Sync`, you will need to first create the Python object and then use +`PyErr::from_instance`. + +Similarly, any types which implemented `PyErrArguments` will now need to be `Send + Sync`. + +#### `PyErr`'s contents are now private + +It is no longer possible to access the fields `.ptype`, `.pvalue` and `.ptraceback` of a `PyErr`. +You should instead now use the new methods `PyErr::ptype()`, `PyErr::pvalue()` and `PyErr::ptraceback()`. + +#### `PyErrValue` and `PyErr::from_value` have been removed + +As these were part the internals of `PyErr` which have been reworked, these APIs no longer exist. + +If you used this API, it is recommended to use `PyException::new_err` (see [the section on +Exception types](#exception-types-have-been-reworked)). + +#### `Into>` for `PyErr` has been removed + +This implementation was redundant. Just construct the `Result::Err` variant directly. + +Before: +```rust,ignore +let result: PyResult<()> = PyErr::new::("error message").into(); +``` + +After (also using the new reworked exception types; see the following section): +```rust +# use pyo3::{PyErr, PyResult, exceptions::PyTypeError}; +let result: PyResult<()> = Err(PyTypeError::new_err("error message")); +``` + +### Exception types have been reworked + +Previously exception types were zero-sized marker types purely used to construct `PyErr`. In PyO3 +0.12, these types have been replaced with full definitions and are usable in the same way as `PyAny`, `PyDict` etc. This +makes it possible to interact with Python exception objects. + +The new types also have names starting with the "Py" prefix. For example, before: + +```rust,ignore +let err: PyErr = TypeError::py_err("error message"); +``` + +After: + +``` +# use pyo3::{PyErr, PyResult, Python, type_object::PyTypeObject}; +# use pyo3::exceptions::{PyBaseException, PyTypeError}; +# Python::with_gil(|py| -> PyResult<()> { +let err: PyErr = PyTypeError::new_err("error message"); + +// Uses Display for PyErr, new for PyO3 0.12 +assert_eq!(err.to_string(), "TypeError: error message"); + +// Now possible to interact with exception instances, new for PyO3 0.12 +let instance: &PyBaseException = err.instance(py); +assert_eq!(instance.getattr("__class__")?, PyTypeError::type_object(py).as_ref()); +# Ok(()) +# }).unwrap(); +``` + ### `FromPy` has been removed -To simplify the PyO3 public conversion trait hierarchy, the `FromPy` has been removed. In PyO3 -`0.11` there were two ways to define the to-Python conversion for a type: `FromPy for PyObject`, -and `IntoPy for T`. +To simplify the PyO3 conversion traits, the `FromPy` trait has been removed. Previously there were +two ways to define the to-Python conversion for a type: +`FromPy for PyObject` and `IntoPy for T`. -Now, the canonical implementation is always `IntoPy`, so downstream crates may need to adjust -accordingly. +Now there is only one way to define the conversion, `IntoPy`, so downstream crates may need to +adjust accordingly. Before: ```rust,ignore @@ -85,7 +160,6 @@ let list_ref: &PyList = list_py.as_ref(py); # }) ``` - ## from 0.10.* to 0.11 ### Stable Rust diff --git a/pyo3-derive-backend/src/from_pyobject.rs b/pyo3-derive-backend/src/from_pyobject.rs index f7688e2b3e4..d35af6da9dd 100644 --- a/pyo3-derive-backend/src/from_pyobject.rs +++ b/pyo3-derive-backend/src/from_pyobject.rs @@ -78,7 +78,7 @@ impl<'a> Enum<'a> { .map(|s| format!("{} ({})", s.to_string_lossy(), type_name)) .unwrap_or_else(|_| type_name.to_string()); let err_msg = format!("Can't convert {} to {}", from, #error_names); - Err(::pyo3::exceptions::PyTypeError::py_err(err_msg)) + Err(::pyo3::exceptions::PyTypeError::new_err(err_msg)) ) } } @@ -263,7 +263,7 @@ impl<'a> Container<'a> { quote!( let s = <::pyo3::types::PyTuple as ::pyo3::conversion::PyTryFrom>::try_from(obj)?; if s.len() != #len { - return Err(::pyo3::exceptions::PyValueError::py_err(#msg)) + return Err(::pyo3::exceptions::PyValueError::new_err(#msg)) } let slice = s.as_slice(); Ok(#self_ty(#fields)) diff --git a/src/buffer.rs b/src/buffer.rs index 4cbb16e1109..2e7c401377f 100644 --- a/src/buffer.rs +++ b/src/buffer.rs @@ -156,10 +156,10 @@ pub unsafe trait Element: Copy { fn validate(b: &ffi::Py_buffer) -> PyResult<()> { // shape and stride information must be provided when we use PyBUF_FULL_RO if b.shape.is_null() { - return Err(exceptions::PyBufferError::py_err("Shape is Null")); + return Err(exceptions::PyBufferError::new_err("Shape is Null")); } if b.strides.is_null() { - return Err(exceptions::PyBufferError::py_err( + return Err(exceptions::PyBufferError::new_err( "PyBuffer: Strides is Null", )); } @@ -190,7 +190,7 @@ impl PyBuffer { { Ok(buf) } else { - Err(exceptions::PyBufferError::py_err( + Err(exceptions::PyBufferError::new_err( "Incompatible type as buffer", )) } @@ -441,7 +441,7 @@ impl PyBuffer { fn copy_to_slice_impl(&self, py: Python, target: &mut [T], fort: u8) -> PyResult<()> { if mem::size_of_val(target) != self.len_bytes() { - return Err(exceptions::PyBufferError::py_err( + return Err(exceptions::PyBufferError::new_err( "Slice length does not match buffer length.", )); } @@ -528,7 +528,7 @@ impl PyBuffer { return buffer_readonly_error(); } if mem::size_of_val(source) != self.len_bytes() { - return Err(exceptions::PyBufferError::py_err( + return Err(exceptions::PyBufferError::new_err( "Slice length does not match buffer length.", )); } @@ -564,7 +564,7 @@ impl PyBuffer { #[inline(always)] fn buffer_readonly_error() -> PyResult<()> { - Err(exceptions::PyBufferError::py_err( + Err(exceptions::PyBufferError::new_err( "Cannot write to read-only buffer.", )) } diff --git a/src/callback.rs b/src/callback.rs index 5f83cfaa96a..2f2632c63c9 100644 --- a/src/callback.rs +++ b/src/callback.rs @@ -86,7 +86,7 @@ impl IntoPyCallbackOutput for usize { if self <= (isize::MAX as usize) { Ok(self as isize) } else { - Err(PyOverflowError::py_err(())) + Err(PyOverflowError::new_err(())) } } } @@ -244,11 +244,11 @@ macro_rules! callback_body_without_convert { Err(e) => { // Try to format the error in the same way panic does if let Some(string) = e.downcast_ref::() { - Err($crate::panic::PanicException::py_err((string.clone(),))) + Err($crate::panic::PanicException::new_err((string.clone(),))) } else if let Some(s) = e.downcast_ref::<&str>() { - Err($crate::panic::PanicException::py_err((s.to_string(),))) + Err($crate::panic::PanicException::new_err((s.to_string(),))) } else { - Err($crate::panic::PanicException::py_err(( + Err($crate::panic::PanicException::new_err(( "panic from Rust code", ))) } diff --git a/src/class/basic.rs b/src/class/basic.rs index d95053bf828..e082405a882 100644 --- a/src/class/basic.rs +++ b/src/class/basic.rs @@ -9,7 +9,7 @@ //! [typeobj docs](https://docs.python.org/3/c-api/typeobj.html) use crate::callback::{HashCallbackOutput, IntoPyCallbackOutput}; -use crate::{exceptions, ffi, FromPyObject, PyAny, PyCell, PyClass, PyErr, PyObject, PyResult}; +use crate::{exceptions, ffi, FromPyObject, PyAny, PyCell, PyClass, PyObject, PyResult}; use std::os::raw::c_int; /// Operators for the __richcmp__ method @@ -260,7 +260,7 @@ where ffi::Py_NE => Ok(CompareOp::Ne), ffi::Py_GT => Ok(CompareOp::Gt), ffi::Py_GE => Ok(CompareOp::Ge), - _ => Err(PyErr::new::( + _ => Err(exceptions::PyValueError::new_err( "tp_richcompare called with invalid comparison operator", )), } diff --git a/src/class/iter.rs b/src/class/iter.rs index 0818feb0a44..057eea57807 100644 --- a/src/class/iter.rs +++ b/src/class/iter.rs @@ -112,7 +112,7 @@ impl IntoPyCallbackOutput<*mut ffi::PyObject> for PyIterNextOutput { fn convert(self, _py: Python) -> PyResult<*mut ffi::PyObject> { match self { IterNextOutput::Yield(o) => Ok(o.into_ptr()), - IterNextOutput::Return(opt) => Err(crate::exceptions::PyStopIteration::py_err((opt,))), + IterNextOutput::Return(opt) => Err(crate::exceptions::PyStopIteration::new_err((opt,))), } } } diff --git a/src/class/macros.rs b/src/class/macros.rs index 2ad6a974ab0..49db13c775b 100644 --- a/src/class/macros.rs +++ b/src/class/macros.rs @@ -228,12 +228,10 @@ macro_rules! py_func_set { let slf = py.from_borrowed_ptr::<$crate::PyCell<$generic>>(slf); if value.is_null() { - Err($crate::PyErr::new::( - format!( - "Subscript deletion not supported by {:?}", - stringify!($generic) - ), - )) + Err($crate::exceptions::PyNotImplementedError::new_err(format!( + "Subscript deletion not supported by {:?}", + stringify!($generic) + ))) } else { let name = py.from_borrowed_ptr::<$crate::PyAny>(name); let value = py.from_borrowed_ptr::<$crate::PyAny>(value); @@ -264,7 +262,7 @@ macro_rules! py_func_del { .extract()?; slf.try_borrow_mut()?.$fn_del(name).convert(py) } else { - Err(PyErr::new::( + Err(exceptions::PyNotImplementedError::new_err( "Subscript assignment not supported", )) } diff --git a/src/class/mapping.rs b/src/class/mapping.rs index d20547e18b8..a8c30651f9c 100644 --- a/src/class/mapping.rs +++ b/src/class/mapping.rs @@ -4,7 +4,6 @@ //! Trait and support implementation for implementing mapping support use crate::callback::IntoPyCallbackOutput; -use crate::err::PyErr; use crate::{exceptions, ffi, FromPyObject, PyClass, PyObject}; /// Mapping interface diff --git a/src/class/pyasync.rs b/src/class/pyasync.rs index 82548835c7f..ddf30b56d3a 100644 --- a/src/class/pyasync.rs +++ b/src/class/pyasync.rs @@ -119,7 +119,7 @@ impl IntoPyCallbackOutput<*mut ffi::PyObject> for PyIterANextOutput { match self { IterANextOutput::Yield(o) => Ok(o.into_ptr()), IterANextOutput::Return(opt) => { - Err(crate::exceptions::PyStopAsyncIteration::py_err((opt,))) + Err(crate::exceptions::PyStopAsyncIteration::new_err((opt,))) } } } diff --git a/src/class/sequence.rs b/src/class/sequence.rs index a9c62f03d0b..d2f29a7f216 100644 --- a/src/class/sequence.rs +++ b/src/class/sequence.rs @@ -223,7 +223,7 @@ mod sq_ass_item_impl { let slf = py.from_borrowed_ptr::>(slf); if value.is_null() { - return Err(PyErr::new::(format!( + return Err(exceptions::PyNotImplementedError::new_err(format!( "Item deletion is not supported by {:?}", stringify!(T) ))); diff --git a/src/derive_utils.rs b/src/derive_utils.rs index 2a736d7ebcd..ead7a42d867 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -43,7 +43,7 @@ pub fn parse_fn_args<'p>( let nargs = args.len(); let mut used_args = 0; macro_rules! raise_error { - ($s: expr $(,$arg:expr)*) => (return Err(PyTypeError::py_err(format!( + ($s: expr $(,$arg:expr)*) => (return Err(PyTypeError::new_err(format!( concat!("{} ", $s), fname.unwrap_or("function") $(,$arg)* )))) } diff --git a/src/err.rs b/src/err.rs deleted file mode 100644 index e1ade1eeb3a..00000000000 --- a/src/err.rs +++ /dev/null @@ -1,673 +0,0 @@ -// Copyright (c) 2017-present PyO3 Project and Contributors - -use crate::gil::ensure_gil; -use crate::panic::PanicException; -use crate::type_object::PyTypeObject; -use crate::types::PyType; -use crate::{exceptions, ffi}; -use crate::{ - AsPyPointer, FromPyPointer, IntoPy, IntoPyPointer, Py, PyAny, PyNativeType, PyObject, Python, - ToBorrowedObject, ToPyObject, -}; -use libc::c_int; -use std::borrow::Cow; -use std::ffi::CString; -use std::io; -use std::os::raw::c_char; -use std::ptr::NonNull; - -/// Represents a `PyErr` value. -/// -/// **Caution:** -/// -/// When you construct an instance of `PyErrValue`, we highly recommend to use `from_err_args` -/// method. If you want to to construct `PyErrValue::ToArgs` directly, please do not forget to -/// call `Python::acquire_gil`. -pub enum PyErrValue { - None, - Value(PyObject), - ToArgs(Box), - ToObject(Box), -} - -impl PyErrValue { - pub fn from_err_args(value: T) -> Self - where - T: PyErrArguments + Send + Sync + 'static, - { - let _ = Python::acquire_gil(); - PyErrValue::ToArgs(Box::new(value)) - } -} - -/// Represents a Python exception that was raised. -pub struct PyErr { - /// The type of the exception. This should be either a `PyClass` or a `PyType`. - pub ptype: Py, - - /// The value of the exception. - /// - /// This can be either an instance of `PyObject`, a tuple of arguments to be passed to - /// `ptype`'s constructor, or a single argument to be passed to `ptype`'s constructor. Call - /// `PyErr::to_object()` to get the exception instance in all cases. - pub pvalue: PyErrValue, - - /// The `PyTraceBack` object associated with the error. - pub ptraceback: Option, -} - -/// Represents the result of a Python call. -pub type PyResult = Result; - -/// Marker type that indicates an error while downcasting -#[derive(Debug)] -pub struct PyDowncastError<'a> { - from: &'a PyAny, - to: Cow<'static, str>, -} - -impl<'a> PyDowncastError<'a> { - pub fn new(from: &'a PyAny, to: impl Into>) -> Self { - PyDowncastError { - from, - to: to.into(), - } - } -} - -/// Helper conversion trait that allows to use custom arguments for exception constructor. -pub trait PyErrArguments { - /// Arguments for exception - fn arguments(&self, _: Python) -> PyObject; -} - -impl PyErr { - /// Creates a new PyErr of type `T`. - /// - /// `value` can be: - /// * a tuple: the exception instance will be created using Python `T(*tuple)` - /// * any other value: the exception instance will be created using Python `T(value)` - /// - /// Note: if `value` is not `Send` or `Sync`, consider using `PyErr::from_value` instead. - /// - /// Panics if `T` is not a Python class derived from `BaseException`. - /// - /// Example: - /// ```ignore - /// return Err(PyErr::new::("Error message")); - /// ``` - /// - /// In most cases, you can use a concrete exception's constructors instead: - /// the example is equivalent to - /// ```ignore - /// return Err(exceptions::PyTypeError::py_err("Error message")); - /// return exceptions::PyTypeError::into("Error message"); - /// ``` - pub fn new(value: V) -> PyErr - where - T: PyTypeObject, - V: ToPyObject + Send + Sync + 'static, - { - let gil = ensure_gil(); - let py = unsafe { gil.python() }; - - let ty = T::type_object(py); - assert_ne!(unsafe { ffi::PyExceptionClass_Check(ty.as_ptr()) }, 0); - - PyErr { - ptype: ty.into(), - pvalue: PyErrValue::ToObject(Box::new(value)), - ptraceback: None, - } - } - - /// Constructs a new error, with the usual lazy initialization of Python exceptions. - /// - /// `exc` is the exception type; usually one of the standard exceptions - /// like `exceptions::PyRuntimeError`. - /// `args` is the a tuple of arguments to pass to the exception constructor. - pub fn from_type(exc: &PyType, args: A) -> PyErr - where - A: ToPyObject + Send + Sync + 'static, - { - PyErr { - ptype: exc.into(), - pvalue: PyErrValue::ToObject(Box::new(args)), - ptraceback: None, - } - } - - /// Creates a new PyErr of type `T`. - pub fn from_value(value: PyErrValue) -> PyErr - where - T: PyTypeObject, - { - let gil = ensure_gil(); - let py = unsafe { gil.python() }; - - let ty = T::type_object(py); - assert_ne!(unsafe { ffi::PyExceptionClass_Check(ty.as_ptr()) }, 0); - - PyErr { - ptype: ty.into(), - pvalue: value, - ptraceback: None, - } - } - - /// Creates a new PyErr. - /// - /// `obj` must be an Python exception instance, the PyErr will use that instance. - /// If `obj` is a Python exception type object, the PyErr will (lazily) create a new - /// instance of that type. - /// Otherwise, a `TypeError` is created instead. - pub fn from_instance(obj: &PyAny) -> PyErr { - let ptr = obj.as_ptr(); - - if unsafe { ffi::PyExceptionInstance_Check(ptr) } != 0 { - PyErr { - ptype: unsafe { - Py::from_borrowed_ptr(obj.py(), ffi::PyExceptionInstance_Class(ptr)) - }, - pvalue: PyErrValue::Value(obj.into()), - ptraceback: None, - } - } else if unsafe { ffi::PyExceptionClass_Check(obj.as_ptr()) } != 0 { - PyErr { - ptype: unsafe { Py::from_borrowed_ptr(obj.py(), ptr) }, - pvalue: PyErrValue::None, - ptraceback: None, - } - } else { - PyErr { - ptype: exceptions::PyTypeError::type_object(obj.py()).into(), - pvalue: PyErrValue::ToObject(Box::new("exceptions must derive from BaseException")), - ptraceback: None, - } - } - } - - /// Gets whether an error is present in the Python interpreter's global state. - #[inline] - pub fn occurred(_: Python) -> bool { - unsafe { !ffi::PyErr_Occurred().is_null() } - } - - /// Retrieves the current error from the Python interpreter's global state. - /// - /// The error is cleared from the Python interpreter. - /// If no error is set, returns a `SystemError`. - /// - /// If the error fetched is a `PanicException` (which would have originated from a panic in a - /// pyo3 callback) then this function will resume the panic. - pub fn fetch(py: Python) -> PyErr { - unsafe { - let mut ptype: *mut ffi::PyObject = std::ptr::null_mut(); - let mut pvalue: *mut ffi::PyObject = std::ptr::null_mut(); - let mut ptraceback: *mut ffi::PyObject = std::ptr::null_mut(); - ffi::PyErr_Fetch(&mut ptype, &mut pvalue, &mut ptraceback); - - let err = PyErr::new_from_ffi_tuple(py, ptype, pvalue, ptraceback); - - if ptype == PanicException::type_object(py).as_ptr() { - let msg: String = PyAny::from_borrowed_ptr_or_opt(py, pvalue) - .and_then(|obj| obj.extract().ok()) - .unwrap_or_else(|| String::from("Unwrapped panic from Python code")); - - eprintln!( - "--- PyO3 is resuming a panic after fetching a PanicException from Python. ---" - ); - eprintln!("Python stack trace below:"); - err.print(py); - - std::panic::resume_unwind(Box::new(msg)) - } - - err - } - } - - /// Creates a new exception type with the given name, which must be of the form - /// `.`, as required by `PyErr_NewException`. - /// - /// `base` can be an existing exception type to subclass, or a tuple of classes - /// `dict` specifies an optional dictionary of class variables and methods - pub fn new_type<'p>( - _: Python<'p>, - name: &str, - base: Option<&PyType>, - dict: Option, - ) -> NonNull { - let base: *mut ffi::PyObject = match base { - None => std::ptr::null_mut(), - Some(obj) => obj.as_ptr(), - }; - - let dict: *mut ffi::PyObject = match dict { - None => std::ptr::null_mut(), - Some(obj) => obj.as_ptr(), - }; - - unsafe { - let null_terminated_name = - CString::new(name).expect("Failed to initialize nul terminated exception name"); - - NonNull::new_unchecked(ffi::PyErr_NewException( - null_terminated_name.as_ptr() as *mut c_char, - base, - dict, - ) as *mut ffi::PyTypeObject) - } - } - - unsafe fn new_from_ffi_tuple( - py: Python, - ptype: *mut ffi::PyObject, - pvalue: *mut ffi::PyObject, - ptraceback: *mut ffi::PyObject, - ) -> PyErr { - // Note: must not panic to ensure all owned pointers get acquired correctly, - // and because we mustn't panic in normalize(). - - let pvalue = if let Some(obj) = PyObject::from_owned_ptr_or_opt(py, pvalue) { - PyErrValue::Value(obj) - } else { - PyErrValue::None - }; - - let ptype = if ptype.is_null() { - ::type_object(py).into() - } else { - Py::from_owned_ptr(py, ptype) - }; - - PyErr { - ptype, - pvalue, - ptraceback: PyObject::from_owned_ptr_or_opt(py, ptraceback), - } - } - - /// Prints a standard traceback to `sys.stderr`. - pub fn print(self, py: Python) { - self.restore(py); - unsafe { ffi::PyErr_PrintEx(0) } - } - - /// Prints a standard traceback to `sys.stderr`, and sets - /// `sys.last_{type,value,traceback}` attributes to this exception's data. - pub fn print_and_set_sys_last_vars(self, py: Python) { - self.restore(py); - unsafe { ffi::PyErr_PrintEx(1) } - } - - /// Returns true if the current exception matches the exception in `exc`. - /// - /// If `exc` is a class object, this also returns `true` when `self` is an instance of a subclass. - /// If `exc` is a tuple, all exceptions in the tuple (and recursively in subtuples) are searched for a match. - pub fn matches(&self, py: Python, exc: T) -> bool - where - T: ToBorrowedObject, - { - exc.with_borrowed_ptr(py, |exc| unsafe { - ffi::PyErr_GivenExceptionMatches(self.ptype.as_ptr(), exc) != 0 - }) - } - - /// Returns true if the current exception is instance of `T`. - pub fn is_instance(&self, py: Python) -> bool - where - T: PyTypeObject, - { - unsafe { - ffi::PyErr_GivenExceptionMatches(self.ptype.as_ptr(), T::type_object(py).as_ptr()) != 0 - } - } - - /// Normalizes the error. This ensures that the exception value is an instance - /// of the exception type. - pub fn normalize(&mut self, py: Python) { - // The normalization helper function involves temporarily moving out of the &mut self, - // which requires some unsafe trickery: - unsafe { - std::ptr::write(self, std::ptr::read(self).into_normalized(py)); - } - // This is safe as long as normalized() doesn't unwind due to a panic. - } - - /// Helper function for normalizing the error by deconstructing and reconstructing the `PyErr`. - /// Must not panic for safety in `normalize()`. - fn into_normalized(self, py: Python) -> PyErr { - let PyErr { - ptype, - pvalue, - ptraceback, - } = self; - - let mut pvalue = match pvalue { - PyErrValue::None => std::ptr::null_mut(), - PyErrValue::Value(ob) => ob.into_ptr(), - PyErrValue::ToArgs(ob) => ob.arguments(py).into_ptr(), - PyErrValue::ToObject(ob) => ob.to_object(py).into_ptr(), - }; - - let mut ptype = ptype.into_ptr(); - let mut ptraceback = ptraceback.into_ptr(); - unsafe { - ffi::PyErr_NormalizeException(&mut ptype, &mut pvalue, &mut ptraceback); - PyErr::new_from_ffi_tuple(py, ptype, pvalue, ptraceback) - } - } - - /// Retrieves the exception instance for this error. - /// - /// This method takes `mut self` because the error might need - /// to be normalized in order to create the exception instance. - pub fn instance(mut self, py: Python) -> &exceptions::PyBaseException { - self.normalize(py); - match self.pvalue { - PyErrValue::Value(ref instance) => { - let any: &PyAny = unsafe { py.from_owned_ptr(instance.clone_ref(py).into_ptr()) }; - any.downcast() - .expect("Normalized error instance should be a BaseException") - } - PyErrValue::None => panic!("This exception is not an instance"), - _ => unreachable!(), - } - } - - /// Writes the error back to the Python interpreter's global state. - /// This is the opposite of `PyErr::fetch()`. - #[inline] - pub fn restore(self, py: Python) { - let PyErr { - ptype, - pvalue, - ptraceback, - } = self; - - let pvalue = match pvalue { - PyErrValue::None => std::ptr::null_mut(), - PyErrValue::Value(ob) => ob.into_ptr(), - PyErrValue::ToArgs(ob) => ob.arguments(py).into_ptr(), - PyErrValue::ToObject(ob) => ob.to_object(py).into_ptr(), - }; - unsafe { ffi::PyErr_Restore(ptype.into_ptr(), pvalue, ptraceback.into_ptr()) } - } - - /// Issues a warning message. - /// May return a `PyErr` if warnings-as-errors is enabled. - pub fn warn(py: Python, category: &PyAny, message: &str, stacklevel: i32) -> PyResult<()> { - let message = CString::new(message)?; - unsafe { - error_on_minusone( - py, - ffi::PyErr_WarnEx( - category.as_ptr(), - message.as_ptr(), - stacklevel as ffi::Py_ssize_t, - ), - ) - } - } - - pub fn clone_ref(&self, py: Python) -> PyErr { - let v = match self.pvalue { - PyErrValue::None => PyErrValue::None, - PyErrValue::Value(ref ob) => PyErrValue::Value(ob.clone_ref(py)), - PyErrValue::ToArgs(ref ob) => PyErrValue::Value(ob.arguments(py)), - PyErrValue::ToObject(ref ob) => PyErrValue::Value(ob.to_object(py)), - }; - - let t = if let Some(ref val) = self.ptraceback { - Some(val.clone_ref(py)) - } else { - None - }; - PyErr { - ptype: self.ptype.clone_ref(py), - pvalue: v, - ptraceback: t, - } - } -} - -impl std::fmt::Debug for PyErr { - fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { - f.write_str(format!("PyErr {{ type: {:?} }}", self.ptype).as_str()) - } -} - -impl IntoPy for PyErr { - fn into_py(self, py: Python) -> PyObject { - self.instance(py).into() - } -} - -impl IntoPy> for PyErr { - fn into_py(self, py: Python) -> Py { - self.instance(py).into() - } -} - -impl ToPyObject for PyErr { - fn to_object(&self, py: Python) -> PyObject { - let err = self.clone_ref(py); - err.instance(py).into() - } -} - -impl<'a> IntoPy for &'a PyErr { - fn into_py(self, py: Python) -> PyObject { - let err = self.clone_ref(py); - err.instance(py).into() - } -} - -/// Convert `PyDowncastError` to Python `TypeError`. -impl<'a> std::convert::From> for PyErr { - fn from(err: PyDowncastError) -> PyErr { - exceptions::PyTypeError::py_err(err.to_string()) - } -} - -impl<'a> std::error::Error for PyDowncastError<'a> {} - -impl<'a> std::fmt::Display for PyDowncastError<'a> { - fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { - write!( - f, - "Can't convert {} to {}", - self.from - .repr() - .map(|s| s.to_string_lossy()) - .unwrap_or_else(|_| self.from.get_type().name()), - self.to - ) - } -} - -/// Convert `PyErr` to `io::Error` -impl std::convert::From for std::io::Error { - fn from(err: PyErr) -> Self { - std::io::Error::new( - std::io::ErrorKind::Other, - format!("Python exception: {:?}", err), - ) - } -} - -/// Convert `PyErr` to `PyResult` -impl std::convert::Into> for PyErr { - fn into(self) -> PyResult { - Err(self) - } -} - -macro_rules! impl_to_pyerr { - ($err: ty, $pyexc: ty) => { - impl PyErrArguments for $err { - fn arguments(&self, py: Python) -> PyObject { - self.to_string().to_object(py) - } - } - - impl std::convert::From<$err> for PyErr { - fn from(err: $err) -> PyErr { - PyErr::from_value::<$pyexc>(PyErrValue::from_err_args(err)) - } - } - }; -} - -/// Create `OSError` from `io::Error` -impl std::convert::From for PyErr { - fn from(err: io::Error) -> PyErr { - macro_rules! err_value { - () => { - PyErrValue::from_err_args(err) - }; - } - match err.kind() { - io::ErrorKind::BrokenPipe => { - PyErr::from_value::(err_value!()) - } - io::ErrorKind::ConnectionRefused => { - PyErr::from_value::(err_value!()) - } - io::ErrorKind::ConnectionAborted => { - PyErr::from_value::(err_value!()) - } - io::ErrorKind::ConnectionReset => { - PyErr::from_value::(err_value!()) - } - io::ErrorKind::Interrupted => { - PyErr::from_value::(err_value!()) - } - io::ErrorKind::NotFound => { - PyErr::from_value::(err_value!()) - } - io::ErrorKind::WouldBlock => { - PyErr::from_value::(err_value!()) - } - io::ErrorKind::TimedOut => { - PyErr::from_value::(err_value!()) - } - _ => PyErr::from_value::(err_value!()), - } - } -} - -impl PyErrArguments for io::Error { - fn arguments(&self, py: Python) -> PyObject { - self.to_string().to_object(py) - } -} - -impl std::convert::From> - for PyErr -{ - fn from(err: std::io::IntoInnerError) -> PyErr { - PyErr::from_value::(PyErrValue::from_err_args(err)) - } -} - -impl PyErrArguments for std::io::IntoInnerError { - fn arguments(&self, py: Python) -> PyObject { - self.to_string().to_object(py) - } -} - -impl PyErrArguments for std::convert::Infallible { - fn arguments(&self, py: Python) -> PyObject { - "Infalliable!".to_object(py) - } -} - -impl std::convert::From for PyErr { - fn from(_: std::convert::Infallible) -> PyErr { - PyErr::new::("Infalliable!") - } -} - -impl_to_pyerr!(std::array::TryFromSliceError, exceptions::PyValueError); -impl_to_pyerr!(std::num::ParseIntError, exceptions::PyValueError); -impl_to_pyerr!(std::num::ParseFloatError, exceptions::PyValueError); -impl_to_pyerr!(std::num::TryFromIntError, exceptions::PyValueError); -impl_to_pyerr!(std::str::ParseBoolError, exceptions::PyValueError); -impl_to_pyerr!(std::ffi::IntoStringError, exceptions::PyUnicodeDecodeError); -impl_to_pyerr!(std::ffi::NulError, exceptions::PyValueError); -impl_to_pyerr!(std::str::Utf8Error, exceptions::PyUnicodeDecodeError); -impl_to_pyerr!(std::string::FromUtf8Error, exceptions::PyUnicodeDecodeError); -impl_to_pyerr!( - std::string::FromUtf16Error, - exceptions::PyUnicodeDecodeError -); -impl_to_pyerr!( - std::char::DecodeUtf16Error, - exceptions::PyUnicodeDecodeError -); -impl_to_pyerr!(std::net::AddrParseError, exceptions::PyValueError); - -pub fn panic_after_error(_py: Python) -> ! { - unsafe { - ffi::PyErr_Print(); - } - panic!("Python API call failed"); -} - -/// Returns Ok if the error code is not -1. -#[inline] -pub fn error_on_minusone(py: Python, result: c_int) -> PyResult<()> { - if result != -1 { - Ok(()) - } else { - Err(PyErr::fetch(py)) - } -} - -#[cfg(test)] -mod tests { - use crate::exceptions; - use crate::panic::PanicException; - use crate::{PyErr, Python}; - - #[test] - fn set_typeerror() { - let gil = Python::acquire_gil(); - let py = gil.python(); - let err: PyErr = exceptions::PyTypeError::py_err(()); - err.restore(py); - assert!(PyErr::occurred(py)); - drop(PyErr::fetch(py)); - } - - #[test] - fn fetching_panic_exception_panics() { - // If -Cpanic=abort is specified, we can't catch panic. - if option_env!("RUSTFLAGS") - .map(|s| s.contains("-Cpanic=abort")) - .unwrap_or(false) - { - return; - } - - let gil = Python::acquire_gil(); - let py = gil.python(); - let err: PyErr = PanicException::py_err("new panic"); - err.restore(py); - assert!(PyErr::occurred(py)); - let started_unwind = - std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| PyErr::fetch(py))).is_err(); - assert!(started_unwind); - } - - #[test] - fn test_pyerr_send_sync() { - fn is_send() {} - fn is_sync() {} - - is_send::(); - is_sync::(); - } -} diff --git a/src/err/err_state.rs b/src/err/err_state.rs new file mode 100644 index 00000000000..a6ae1260956 --- /dev/null +++ b/src/err/err_state.rs @@ -0,0 +1,69 @@ +use crate::{ + exceptions::PyBaseException, ffi, types::PyType, IntoPy, IntoPyPointer, Py, PyObject, Python, +}; + +#[derive(Clone)] +pub(crate) struct PyErrStateNormalized { + pub ptype: Py, + pub pvalue: Py, + pub ptraceback: Option, +} + +pub(crate) enum PyErrState { + Lazy { + ptype: Py, + pvalue: Box PyObject + Send + Sync>, + }, + FfiTuple { + ptype: Option, + pvalue: Option, + ptraceback: Option, + }, + Normalized(PyErrStateNormalized), +} + +/// Helper conversion trait that allows to use custom arguments for lazy exception construction. +pub trait PyErrArguments: Send + Sync { + /// Arguments for exception + fn arguments(self, py: Python) -> PyObject; +} + +impl PyErrArguments for T +where + T: IntoPy + Send + Sync, +{ + fn arguments(self, py: Python) -> PyObject { + self.into_py(py) + } +} + +pub(crate) fn boxed_args( + args: impl PyErrArguments + 'static, +) -> Box PyObject + Send + Sync> { + Box::new(|py| args.arguments(py)) +} + +impl PyErrState { + pub(crate) fn into_ffi_tuple( + self, + py: Python, + ) -> (*mut ffi::PyObject, *mut ffi::PyObject, *mut ffi::PyObject) { + match self { + PyErrState::Lazy { ptype, pvalue } => ( + ptype.into_ptr(), + pvalue(py).into_ptr(), + std::ptr::null_mut(), + ), + PyErrState::FfiTuple { + ptype, + pvalue, + ptraceback, + } => (ptype.into_ptr(), pvalue.into_ptr(), ptraceback.into_ptr()), + PyErrState::Normalized(PyErrStateNormalized { + ptype, + pvalue, + ptraceback, + }) => (ptype.into_ptr(), pvalue.into_ptr(), ptraceback.into_ptr()), + } + } +} diff --git a/src/err/impls.rs b/src/err/impls.rs new file mode 100644 index 00000000000..77dc16d8149 --- /dev/null +++ b/src/err/impls.rs @@ -0,0 +1,114 @@ +use crate::{err::PyErrArguments, exceptions, IntoPy, PyErr, PyObject, Python}; +use std::io; + +/// Convert `PyErr` to `io::Error` +impl std::convert::From for io::Error { + fn from(err: PyErr) -> Self { + io::Error::new(io::ErrorKind::Other, format!("Python exception: {}", err)) + } +} + +/// Create `OSError` from `io::Error` +impl std::convert::From for PyErr { + fn from(err: io::Error) -> PyErr { + match err.kind() { + io::ErrorKind::BrokenPipe => exceptions::PyBrokenPipeError::new_err(err), + io::ErrorKind::ConnectionRefused => exceptions::PyConnectionRefusedError::new_err(err), + io::ErrorKind::ConnectionAborted => exceptions::PyConnectionAbortedError::new_err(err), + io::ErrorKind::ConnectionReset => exceptions::PyConnectionResetError::new_err(err), + io::ErrorKind::Interrupted => exceptions::PyInterruptedError::new_err(err), + io::ErrorKind::NotFound => exceptions::PyFileNotFoundError::new_err(err), + io::ErrorKind::WouldBlock => exceptions::PyBlockingIOError::new_err(err), + io::ErrorKind::TimedOut => exceptions::PyTimeoutError::new_err(err), + _ => exceptions::PyOSError::new_err(err), + } + } +} + +impl PyErrArguments for io::Error { + fn arguments(self, py: Python) -> PyObject { + self.to_string().into_py(py) + } +} + +impl std::convert::From> + for PyErr +{ + fn from(err: std::io::IntoInnerError) -> PyErr { + exceptions::PyOSError::new_err(err) + } +} + +impl PyErrArguments for std::io::IntoInnerError { + fn arguments(self, py: Python) -> PyObject { + self.to_string().into_py(py) + } +} + +impl std::convert::From for PyErr { + fn from(_: std::convert::Infallible) -> PyErr { + unreachable!() + } +} + +macro_rules! impl_to_pyerr { + ($err: ty, $pyexc: ty) => { + impl PyErrArguments for $err { + fn arguments(self, py: Python) -> PyObject { + self.to_string().into_py(py) + } + } + + impl std::convert::From<$err> for PyErr { + fn from(err: $err) -> PyErr { + <$pyexc>::new_err(err) + } + } + }; +} + +impl_to_pyerr!(std::array::TryFromSliceError, exceptions::PyValueError); +impl_to_pyerr!(std::num::ParseIntError, exceptions::PyValueError); +impl_to_pyerr!(std::num::ParseFloatError, exceptions::PyValueError); +impl_to_pyerr!(std::num::TryFromIntError, exceptions::PyValueError); +impl_to_pyerr!(std::str::ParseBoolError, exceptions::PyValueError); +impl_to_pyerr!(std::ffi::IntoStringError, exceptions::PyUnicodeDecodeError); +impl_to_pyerr!(std::ffi::NulError, exceptions::PyValueError); +impl_to_pyerr!(std::str::Utf8Error, exceptions::PyUnicodeDecodeError); +impl_to_pyerr!(std::string::FromUtf8Error, exceptions::PyUnicodeDecodeError); +impl_to_pyerr!( + std::string::FromUtf16Error, + exceptions::PyUnicodeDecodeError +); +impl_to_pyerr!( + std::char::DecodeUtf16Error, + exceptions::PyUnicodeDecodeError +); +impl_to_pyerr!(std::net::AddrParseError, exceptions::PyValueError); + +#[cfg(test)] +mod tests { + use crate::PyErr; + use std::io; + + #[test] + fn io_errors() { + let check_err = |kind, expected_ty| { + let py_err: PyErr = io::Error::new(kind, "some error msg").into(); + let err_msg = format!("{}: some error msg", expected_ty); + assert_eq!(py_err.to_string(), err_msg); + + let os_err: io::Error = py_err.into(); + assert_eq!(os_err.to_string(), format!("Python exception: {}", err_msg)); + }; + + check_err(io::ErrorKind::BrokenPipe, "BrokenPipeError"); + check_err(io::ErrorKind::ConnectionRefused, "ConnectionRefusedError"); + check_err(io::ErrorKind::ConnectionAborted, "ConnectionAbortedError"); + check_err(io::ErrorKind::ConnectionReset, "ConnectionResetError"); + check_err(io::ErrorKind::Interrupted, "InterruptedError"); + check_err(io::ErrorKind::NotFound, "FileNotFoundError"); + check_err(io::ErrorKind::WouldBlock, "BlockingIOError"); + check_err(io::ErrorKind::TimedOut, "TimeoutError"); + } +} diff --git a/src/err/mod.rs b/src/err/mod.rs new file mode 100644 index 00000000000..724ce0caecf --- /dev/null +++ b/src/err/mod.rs @@ -0,0 +1,576 @@ +// Copyright (c) 2017-present PyO3 Project and Contributors + +use crate::panic::PanicException; +use crate::type_object::PyTypeObject; +use crate::types::PyType; +use crate::{ + exceptions::{self, PyBaseException}, + ffi, +}; +use crate::{ + AsPyPointer, FromPyPointer, IntoPy, Py, PyAny, PyNativeType, PyObject, Python, + ToBorrowedObject, ToPyObject, +}; +use libc::c_int; +use std::borrow::Cow; +use std::cell::UnsafeCell; +use std::ffi::CString; +use std::os::raw::c_char; +use std::ptr::NonNull; + +mod err_state; +mod impls; + +pub use err_state::PyErrArguments; +use err_state::{boxed_args, PyErrState, PyErrStateNormalized}; + +/// Represents a Python exception that was raised. +pub struct PyErr { + // Safety: can only hand out references when in the "normalized" state. Will never change + // after normalization. + // + // The state is temporarily removed from the PyErr during normalization, to avoid + // concurrent modifications. + state: UnsafeCell>, +} + +unsafe impl Send for PyErr {} +unsafe impl Sync for PyErr {} + +/// Represents the result of a Python call. +pub type PyResult = Result; + +/// Marker type that indicates an error while downcasting +#[derive(Debug)] +pub struct PyDowncastError<'a> { + from: &'a PyAny, + to: Cow<'static, str>, +} + +impl<'a> PyDowncastError<'a> { + pub fn new(from: &'a PyAny, to: impl Into>) -> Self { + PyDowncastError { + from, + to: to.into(), + } + } +} + +impl PyErr { + /// Creates a new PyErr of type `T`. + /// + /// `value` can be: + /// * a tuple: the exception instance will be created using Python `T(*tuple)` + /// * any other value: the exception instance will be created using Python `T(value)` + /// + /// Note: if `value` is not `Send` or `Sync`, consider using `PyErr::from_instance` instead. + /// + /// Panics if `T` is not a Python class derived from `BaseException`. + /// + /// Example: + /// ```ignore + /// return Err(PyErr::new::("Error message")); + /// ``` + /// + /// In most cases, you can use a concrete exception's constructor instead, which is equivalent: + /// ```ignore + /// return Err(exceptions::PyTypeError::new_err("Error message")); + /// ``` + pub fn new(args: A) -> PyErr + where + T: PyTypeObject, + A: PyErrArguments + Send + Sync + 'static, + { + Python::with_gil(|py| PyErr::from_type(T::type_object(py), args)) + } + + /// Constructs a new error, with the usual lazy initialization of Python exceptions. + /// + /// `exc` is the exception type; usually one of the standard exceptions + /// like `exceptions::PyRuntimeError`. + /// `args` is the a tuple of arguments to pass to the exception constructor. + pub fn from_type(ty: &PyType, args: A) -> PyErr + where + A: PyErrArguments + Send + Sync + 'static, + { + if unsafe { ffi::PyExceptionClass_Check(ty.as_ptr()) } == 0 { + return exceptions_must_derive_from_base_exception(ty.py()); + } + + PyErr::from_state(PyErrState::Lazy { + ptype: ty.into(), + pvalue: boxed_args(args), + }) + } + + /// Creates a new PyErr. + /// + /// `obj` must be an Python exception instance, the PyErr will use that instance. + /// If `obj` is a Python exception type object, the PyErr will (lazily) create a new + /// instance of that type. + /// Otherwise, a `TypeError` is created instead. + /// + /// # Example + /// ```rust + /// use pyo3::{Python, PyErr, IntoPy, exceptions::PyTypeError, types::PyType}; + /// Python::with_gil(|py| { + /// // Case #1: Exception instance + /// let err = PyErr::from_instance(PyTypeError::new_err("some type error",).instance(py)); + /// assert_eq!(err.to_string(), "TypeError: some type error"); + /// + /// // Case #2: Exception type + /// let err = PyErr::from_instance(PyType::new::(py)); + /// assert_eq!(err.to_string(), "TypeError: "); + /// + /// // Case #3: Invalid exception value + /// let err = PyErr::from_instance("foo".into_py(py).as_ref(py)); + /// assert_eq!(err.to_string(), "TypeError: exceptions must derive from BaseException"); + /// }); + /// ``` + pub fn from_instance(obj: &PyAny) -> PyErr { + let ptr = obj.as_ptr(); + + let state = if unsafe { ffi::PyExceptionInstance_Check(ptr) } != 0 { + PyErrState::Normalized(PyErrStateNormalized { + ptype: unsafe { + Py::from_borrowed_ptr(obj.py(), ffi::PyExceptionInstance_Class(ptr)) + }, + pvalue: unsafe { Py::from_borrowed_ptr(obj.py(), obj.as_ptr()) }, + ptraceback: None, + }) + } else if unsafe { ffi::PyExceptionClass_Check(obj.as_ptr()) } != 0 { + PyErrState::FfiTuple { + ptype: unsafe { Some(Py::from_borrowed_ptr(obj.py(), ptr)) }, + pvalue: None, + ptraceback: None, + } + } else { + return exceptions_must_derive_from_base_exception(obj.py()); + }; + + PyErr::from_state(state) + } + + /// Get the type of this exception object. + /// + /// The object will be normalized first if needed. + /// + /// # Example + /// ```rust + /// use pyo3::{Python, PyErr, exceptions::PyTypeError, types::PyType}; + /// Python::with_gil(|py| { + /// let err = PyTypeError::new_err(("some type error",)); + /// assert_eq!(err.ptype(py), PyType::new::(py)); + /// }); + /// ``` + pub fn ptype<'py>(&'py self, py: Python<'py>) -> &'py PyType { + self.normalized(py).ptype.as_ref(py) + } + + /// Get the value of this exception object. + /// + /// The object will be normalized first if needed. + /// + /// # Example + /// ```rust + /// use pyo3::{Python, PyErr, exceptions::PyTypeError, types::PyType}; + /// Python::with_gil(|py| { + /// let err = PyTypeError::new_err(("some type error",)); + /// assert_eq!(err.pvalue(py).to_string(), "TypeError: some type error"); + /// }); + /// ``` + pub fn pvalue<'py>(&'py self, py: Python<'py>) -> &'py PyBaseException { + self.normalized(py).pvalue.as_ref(py) + } + + /// Get the value of this exception object. + /// + /// The object will be normalized first if needed. + /// + /// # Example + /// ```rust + /// use pyo3::{Python, PyErr, exceptions::PyTypeError, types::PyType}; + /// Python::with_gil(|py| { + /// let err = PyTypeError::new_err(("some type error",)); + /// assert_eq!(err.ptraceback(py), None); + /// }); + /// ``` + pub fn ptraceback<'py>(&'py self, py: Python<'py>) -> Option<&'py PyAny> { + self.normalized(py) + .ptraceback + .as_ref() + .map(|obj| obj.as_ref(py)) + } + + /// Gets whether an error is present in the Python interpreter's global state. + #[inline] + pub fn occurred(_: Python) -> bool { + unsafe { !ffi::PyErr_Occurred().is_null() } + } + + /// Retrieves the current error from the Python interpreter's global state. + /// + /// The error is cleared from the Python interpreter. + /// If no error is set, returns a `SystemError`. + /// + /// If the error fetched is a `PanicException` (which would have originated from a panic in a + /// pyo3 callback) then this function will resume the panic. + pub fn fetch(py: Python) -> PyErr { + unsafe { + let mut ptype: *mut ffi::PyObject = std::ptr::null_mut(); + let mut pvalue: *mut ffi::PyObject = std::ptr::null_mut(); + let mut ptraceback: *mut ffi::PyObject = std::ptr::null_mut(); + ffi::PyErr_Fetch(&mut ptype, &mut pvalue, &mut ptraceback); + + let err = PyErr::new_from_ffi_tuple(py, ptype, pvalue, ptraceback); + + if ptype == PanicException::type_object(py).as_ptr() { + let msg: String = PyAny::from_borrowed_ptr_or_opt(py, pvalue) + .and_then(|obj| obj.extract().ok()) + .unwrap_or_else(|| String::from("Unwrapped panic from Python code")); + + eprintln!( + "--- PyO3 is resuming a panic after fetching a PanicException from Python. ---" + ); + eprintln!("Python stack trace below:"); + err.print(py); + + std::panic::resume_unwind(Box::new(msg)) + } + + err + } + } + + /// Creates a new exception type with the given name, which must be of the form + /// `.`, as required by `PyErr_NewException`. + /// + /// `base` can be an existing exception type to subclass, or a tuple of classes + /// `dict` specifies an optional dictionary of class variables and methods + pub fn new_type<'p>( + _: Python<'p>, + name: &str, + base: Option<&PyType>, + dict: Option, + ) -> NonNull { + let base: *mut ffi::PyObject = match base { + None => std::ptr::null_mut(), + Some(obj) => obj.as_ptr(), + }; + + let dict: *mut ffi::PyObject = match dict { + None => std::ptr::null_mut(), + Some(obj) => obj.as_ptr(), + }; + + unsafe { + let null_terminated_name = + CString::new(name).expect("Failed to initialize nul terminated exception name"); + + NonNull::new_unchecked(ffi::PyErr_NewException( + null_terminated_name.as_ptr() as *mut c_char, + base, + dict, + ) as *mut ffi::PyTypeObject) + } + } + + /// Create a PyErr from an ffi tuple + /// + /// # Safety + /// - `ptype` must be a pointer to valid Python exception type object. + /// - `pvalue` must be a pointer to a valid Python object, or NULL. + /// - `ptraceback` must be a pointer to a valid Python traceback object, or NULL. + unsafe fn new_from_ffi_tuple( + py: Python, + ptype: *mut ffi::PyObject, + pvalue: *mut ffi::PyObject, + ptraceback: *mut ffi::PyObject, + ) -> PyErr { + // Note: must not panic to ensure all owned pointers get acquired correctly, + // and because we mustn't panic in normalize(). + PyErr::from_state(PyErrState::FfiTuple { + ptype: Py::from_owned_ptr_or_opt(py, ptype), + pvalue: Py::from_owned_ptr_or_opt(py, pvalue), + ptraceback: Py::from_owned_ptr_or_opt(py, ptraceback), + }) + } + + /// Prints a standard traceback to `sys.stderr`. + pub fn print(&self, py: Python) { + self.clone_ref(py).restore(py); + unsafe { ffi::PyErr_PrintEx(0) } + } + + /// Prints a standard traceback to `sys.stderr`, and sets + /// `sys.last_{type,value,traceback}` attributes to this exception's data. + pub fn print_and_set_sys_last_vars(&self, py: Python) { + self.clone_ref(py).restore(py); + unsafe { ffi::PyErr_PrintEx(1) } + } + + /// Returns true if the current exception matches the exception in `exc`. + /// + /// If `exc` is a class object, this also returns `true` when `self` is an instance of a subclass. + /// If `exc` is a tuple, all exceptions in the tuple (and recursively in subtuples) are searched for a match. + pub fn matches(&self, py: Python, exc: T) -> bool + where + T: ToBorrowedObject, + { + exc.with_borrowed_ptr(py, |exc| unsafe { + ffi::PyErr_GivenExceptionMatches(self.ptype_ptr(), exc) != 0 + }) + } + + /// Returns true if the current exception is instance of `T`. + pub fn is_instance(&self, py: Python) -> bool + where + T: PyTypeObject, + { + unsafe { + ffi::PyErr_GivenExceptionMatches(self.ptype_ptr(), T::type_object(py).as_ptr()) != 0 + } + } + + /// Retrieves the exception instance for this error. + pub fn instance<'py>(&'py self, py: Python<'py>) -> &'py PyBaseException { + self.normalized(py).pvalue.as_ref(py) + } + + /// Consumes self to take ownership of the exception instance for this error. + pub fn into_instance(self, py: Python) -> Py { + let out = self.normalized(py).pvalue.as_ref(py).into(); + std::mem::forget(self); + out + } + + /// Writes the error back to the Python interpreter's global state. + /// This is the opposite of `PyErr::fetch()`. + #[inline] + pub fn restore(self, py: Python) { + let (ptype, pvalue, ptraceback) = self + .state + .into_inner() + .expect("Cannot restore a PyErr while normalizing it") + .into_ffi_tuple(py); + unsafe { ffi::PyErr_Restore(ptype, pvalue, ptraceback) } + } + + /// Issues a warning message. + /// May return a `PyErr` if warnings-as-errors is enabled. + pub fn warn(py: Python, category: &PyAny, message: &str, stacklevel: i32) -> PyResult<()> { + let message = CString::new(message)?; + unsafe { + error_on_minusone( + py, + ffi::PyErr_WarnEx( + category.as_ptr(), + message.as_ptr(), + stacklevel as ffi::Py_ssize_t, + ), + ) + } + } + + /// Clone the PyErr. This requires the GIL, which is why PyErr does not implement Clone. + /// + /// # Example + /// ```rust + /// use pyo3::{Python, PyErr, exceptions::PyTypeError, types::PyType}; + /// Python::with_gil(|py| { + /// let err = PyTypeError::new_err(("some type error",)); + /// let err_clone = err.clone_ref(py); + /// assert_eq!(err.ptype(py), err_clone.ptype(py)); + /// assert_eq!(err.pvalue(py), err_clone.pvalue(py)); + /// assert_eq!(err.ptraceback(py), err_clone.ptraceback(py)); + /// }); + /// ``` + pub fn clone_ref(&self, py: Python) -> PyErr { + PyErr::from_state(PyErrState::Normalized(self.normalized(py).clone())) + } + + fn from_state(state: PyErrState) -> PyErr { + PyErr { + state: UnsafeCell::new(Some(state)), + } + } + + /// Returns borrowed reference to this Err's type + fn ptype_ptr(&self) -> *mut ffi::PyObject { + match unsafe { &*self.state.get() } { + Some(PyErrState::Lazy { ptype, .. }) => ptype.as_ptr(), + Some(PyErrState::FfiTuple { ptype, .. }) => ptype.as_ptr(), + Some(PyErrState::Normalized(n)) => n.ptype.as_ptr(), + None => panic!("Cannot access exception type while normalizing"), + } + } + + fn normalized(&self, py: Python) -> &PyErrStateNormalized { + // This process is safe because: + // - Access is guaranteed not to be concurrent thanks to `Python` GIL token + // - Write happens only once, and then never will change again. + // - State is set to None during the normalization process, so that a second + // concurrent normalization attempt will panic before changing anything. + + if let Some(PyErrState::Normalized(n)) = unsafe { &*self.state.get() } { + return n; + } + + let state = unsafe { + (*self.state.get()) + .take() + .expect("Cannot normalize a PyErr while already normalizing it.") + }; + let (mut ptype, mut pvalue, mut ptraceback) = state.into_ffi_tuple(py); + + unsafe { + ffi::PyErr_NormalizeException(&mut ptype, &mut pvalue, &mut ptraceback); + let self_state = &mut *self.state.get(); + *self_state = Some(PyErrState::Normalized(PyErrStateNormalized { + ptype: Py::from_owned_ptr_or_opt(py, ptype) + .unwrap_or_else(|| exceptions::PySystemError::type_object(py).into()), + pvalue: Py::from_owned_ptr_or_opt(py, pvalue).unwrap_or_else(|| { + exceptions::PySystemError::new_err("Exception value missing") + .instance(py) + .into_py(py) + }), + ptraceback: PyObject::from_owned_ptr_or_opt(py, ptraceback), + })); + + match self_state { + Some(PyErrState::Normalized(n)) => n, + _ => unreachable!(), + } + } + } +} + +impl std::fmt::Debug for PyErr { + fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { + f.write_str(format!("PyErr {{ type: {:?} }}", self.ptype_ptr()).as_str()) + } +} + +impl std::fmt::Display for PyErr { + fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { + Python::with_gil(|py| self.instance(py).fmt(f)) + } +} + +impl std::error::Error for PyErr {} + +impl IntoPy for PyErr { + fn into_py(self, py: Python) -> PyObject { + self.into_instance(py).into() + } +} + +impl ToPyObject for PyErr { + fn to_object(&self, py: Python) -> PyObject { + self.clone_ref(py).into_py(py) + } +} + +impl<'a> IntoPy for &'a PyErr { + fn into_py(self, py: Python) -> PyObject { + self.clone_ref(py).into_py(py) + } +} + +/// Convert `PyDowncastError` to Python `TypeError`. +impl<'a> std::convert::From> for PyErr { + fn from(err: PyDowncastError) -> PyErr { + exceptions::PyTypeError::new_err(err.to_string()) + } +} + +impl<'a> std::error::Error for PyDowncastError<'a> {} + +impl<'a> std::fmt::Display for PyDowncastError<'a> { + fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { + write!( + f, + "Can't convert {} to {}", + self.from + .repr() + .map(|s| s.to_string_lossy()) + .unwrap_or_else(|_| self.from.get_type().name()), + self.to + ) + } +} + +pub fn panic_after_error(_py: Python) -> ! { + unsafe { + ffi::PyErr_Print(); + } + panic!("Python API call failed"); +} + +/// Returns Ok if the error code is not -1. +#[inline] +pub fn error_on_minusone(py: Python, result: c_int) -> PyResult<()> { + if result != -1 { + Ok(()) + } else { + Err(PyErr::fetch(py)) + } +} + +#[inline] +fn exceptions_must_derive_from_base_exception(py: Python) -> PyErr { + PyErr::from_state(PyErrState::Lazy { + ptype: exceptions::PyTypeError::type_object(py).into(), + pvalue: boxed_args("exceptions must derive from BaseException"), + }) +} + +#[cfg(test)] +mod tests { + use super::PyErrState; + use crate::exceptions; + use crate::panic::PanicException; + use crate::{PyErr, Python}; + + #[test] + fn set_typeerror() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let err: PyErr = exceptions::PyTypeError::new_err(()); + err.restore(py); + assert!(PyErr::occurred(py)); + drop(PyErr::fetch(py)); + } + + #[test] + fn fetching_panic_exception_panics() { + // If -Cpanic=abort is specified, we can't catch panic. + if option_env!("RUSTFLAGS") + .map(|s| s.contains("-Cpanic=abort")) + .unwrap_or(false) + { + return; + } + + let gil = Python::acquire_gil(); + let py = gil.python(); + let err: PyErr = PanicException::new_err("new panic"); + err.restore(py); + assert!(PyErr::occurred(py)); + let started_unwind = + std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| PyErr::fetch(py))).is_err(); + assert!(started_unwind); + } + + #[test] + fn test_pyerr_send_sync() { + fn is_send() {} + fn is_sync() {} + + is_send::(); + is_sync::(); + + is_send::(); + is_sync::(); + } +} diff --git a/src/exceptions.rs b/src/exceptions.rs index be73c5d4bcc..b748b923245 100644 --- a/src/exceptions.rs +++ b/src/exceptions.rs @@ -3,36 +3,26 @@ //! Exception types defined by Python. use crate::type_object::PySizedLayout; -use crate::types::{PyAny, PyTuple}; -use crate::{ffi, AsPyPointer, PyResult, Python}; +use crate::{ffi, PyResult, Python}; use std::ffi::CStr; use std::ops; -use std::os::raw::c_char; /// The boilerplate to convert between a Rust type and a Python exception. #[macro_export] macro_rules! impl_exception_boilerplate { ($name: ident) => { - impl std::convert::From<$name> for $crate::PyErr { - fn from(_err: $name) -> $crate::PyErr { - $crate::PyErr::new::<$name, _>(()) - } - } - - impl std::convert::Into<$crate::PyResult> for $name { - fn into(self) -> $crate::PyResult { - $crate::PyErr::new::<$name, _>(()).into() + impl std::convert::From<&$name> for $crate::PyErr { + fn from(err: &$name) -> $crate::PyErr { + $crate::PyErr::from_instance(err) } } impl $name { - pub fn py_err(args: V) -> $crate::PyErr { - $crate::PyErr::new::<$name, V>(args) - } - pub fn into( - args: V, - ) -> $crate::PyResult { - $crate::PyErr::new::<$name, V>(args).into() + pub fn new_err(args: A) -> $crate::PyErr + where + A: $crate::PyErrArguments + Send + Sync + 'static, + { + $crate::PyErr::new::<$name, A>(args) } } @@ -445,18 +435,17 @@ impl_native_exception!(PyIOError, IOError, PyExc_IOError); impl_native_exception!(PyWindowsError, WindowsError, PyExc_WindowsError); impl PyUnicodeDecodeError { - pub fn new_err<'p>( + pub fn new<'p>( py: Python<'p>, encoding: &CStr, input: &[u8], range: ops::Range, reason: &CStr, - ) -> PyResult<&'p PyAny> { + ) -> PyResult<&'p PyUnicodeDecodeError> { unsafe { - let input: &[c_char] = &*(input as *const [u8] as *const [c_char]); py.from_owned_ptr_or_err(ffi::PyUnicodeDecodeError_Create( encoding.as_ptr(), - input.as_ptr(), + input.as_ptr() as *const i8, input.len() as ffi::Py_ssize_t, range.start as ffi::Py_ssize_t, range.end as ffi::Py_ssize_t, @@ -465,34 +454,22 @@ impl PyUnicodeDecodeError { } } - #[allow(clippy::range_plus_one)] // False positive, ..= returns the wrong type pub fn new_utf8<'p>( py: Python<'p>, input: &[u8], err: std::str::Utf8Error, - ) -> PyResult<&'p PyAny> { + ) -> PyResult<&'p PyUnicodeDecodeError> { let pos = err.valid_up_to(); - PyUnicodeDecodeError::new_err( + PyUnicodeDecodeError::new( py, CStr::from_bytes_with_nul(b"utf-8\0").unwrap(), input, - pos..pos + 1, + pos..(pos + 1), CStr::from_bytes_with_nul(b"invalid utf-8\0").unwrap(), ) } } -impl PyStopIteration { - pub fn stop_iteration(_py: Python, args: &PyTuple) { - unsafe { - ffi::PyErr_SetObject( - ffi::PyExc_StopIteration as *mut ffi::PyObject, - args.as_ptr(), - ); - } - } -} - /// Exceptions defined in `asyncio` module pub mod asyncio { import_exception!(asyncio, CancelledError); @@ -513,11 +490,10 @@ pub mod socket { #[cfg(test)] mod test { - use crate::exceptions::PyException; + use super::{PyException, PyUnicodeDecodeError}; use crate::types::{IntoPyDict, PyDict}; - use crate::{AsPyPointer, PyErr, Python}; + use crate::{PyErr, Python}; use std::error::Error; - use std::fmt::Write; import_exception!(socket, gaierror); import_exception!(email.errors, MessageError); @@ -527,7 +503,7 @@ mod test { let gil = Python::acquire_gil(); let py = gil.python(); - let err: PyErr = gaierror::py_err(()); + let err: PyErr = gaierror::new_err(()); let socket = py .import("socket") .map_err(|e| e.print(py)) @@ -537,6 +513,7 @@ mod test { d.set_item("socket", socket) .map_err(|e| e.print(py)) .expect("could not setitem"); + d.set_item("exc", err) .map_err(|e| e.print(py)) .expect("could not setitem"); @@ -551,7 +528,7 @@ mod test { let gil = Python::acquire_gil(); let py = gil.python(); - let err: PyErr = MessageError::py_err(()); + let err: PyErr = MessageError::new_err(()); let email = py .import("email") .map_err(|e| e.print(py)) @@ -596,41 +573,66 @@ mod test { .unwrap(); } + #[test] + fn native_exception_debug() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let exc = py + .run("raise Exception('banana')", None, None) + .expect_err("raising should have given us an error") + .into_instance(py); + assert_eq!(format!("{:?}", exc.as_ref(py)), "Exception"); + } + #[test] fn native_exception_display() { - let mut out = String::new(); let gil = Python::acquire_gil(); let py = gil.python(); - let err = py + let exc = py .run("raise Exception('banana')", None, None) .expect_err("raising should have given us an error") - .instance(py); - write!(&mut out, "{}", err).expect("successful format"); - assert_eq!(out, "Exception: banana"); + .into_instance(py); + assert_eq!(exc.to_string(), "Exception: banana"); } #[test] fn native_exception_chain() { - let mut out = String::new(); let gil = Python::acquire_gil(); let py = gil.python(); - let err = py + let exc = py .run( "raise Exception('banana') from TypeError('peach')", None, None, ) .expect_err("raising should have given us an error") - .instance(py); - write!(&mut out, "{}", err).expect("successful format"); - assert_eq!(out, "Exception: banana"); - out.clear(); - let convert_ref: &super::PyBaseException = - unsafe { &*(err.as_ptr() as *const _ as *const _) }; - let source = convert_ref.source().expect("cause should exist"); - write!(&mut out, "{}", source).expect("successful format"); - assert_eq!(out, "TypeError: peach"); + .into_instance(py); + assert_eq!(exc.to_string(), "Exception: banana"); + let source = exc.as_ref(py).source().expect("cause should exist"); + assert_eq!(source.to_string(), "TypeError: peach"); let source_source = source.source(); assert!(source_source.is_none(), "source_source should be None"); } + + #[test] + fn unicode_decode_error() { + let invalid_utf8 = b"fo\xd8o"; + let err = std::str::from_utf8(invalid_utf8).expect_err("should be invalid utf8"); + Python::with_gil(|py| { + let decode_err = PyUnicodeDecodeError::new_utf8(py, invalid_utf8, err).unwrap(); + assert_eq!( + decode_err.to_string(), + "UnicodeDecodeError: \'utf-8\' codec can\'t decode byte 0xd8 in position 2: invalid utf-8" + ); + + // Restoring should preserve the same error + let e: PyErr = decode_err.into(); + e.restore(py); + + assert_eq!( + PyErr::fetch(py).to_string(), + "UnicodeDecodeError: \'utf-8\' codec can\'t decode byte 0xd8 in position 2: invalid utf-8" + ); + }); + } } diff --git a/src/gil.rs b/src/gil.rs index 989f1540d0d..fb7851e8b37 100644 --- a/src/gil.rs +++ b/src/gil.rs @@ -365,10 +365,8 @@ fn decrement_gil_count() { }); } -/// Ensure the GIL is held, useful in implementation of APIs like PyErr::new where it's -/// inconvenient to force the user to acquire the GIL. -#[doc(hidden)] -pub fn ensure_gil() -> EnsureGIL { +/// Ensure the GIL is held, used in the implementation of Python::with_gil +pub(crate) fn ensure_gil() -> EnsureGIL { if gil_is_acquired() { EnsureGIL(None) } else { @@ -377,8 +375,7 @@ pub fn ensure_gil() -> EnsureGIL { } /// Struct used internally which avoids acquiring the GIL where it's not necessary. -#[doc(hidden)] -pub struct EnsureGIL(Option); +pub(crate) struct EnsureGIL(Option); impl EnsureGIL { /// Get the GIL token. diff --git a/src/lib.rs b/src/lib.rs index 10f3e768f8e..8a9665dddb6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -138,7 +138,7 @@ pub use crate::conversion::{ AsPyPointer, FromPyObject, FromPyPointer, IntoPy, IntoPyPointer, PyTryFrom, PyTryInto, ToBorrowedObject, ToPyObject, }; -pub use crate::err::{PyDowncastError, PyErr, PyErrArguments, PyErrValue, PyResult}; +pub use crate::err::{PyDowncastError, PyErr, PyErrArguments, PyResult}; pub use crate::gil::{GILGuard, GILPool}; pub use crate::instance::{Py, PyNativeType, PyObject}; pub use crate::pycell::{PyCell, PyRef, PyRefMut}; @@ -196,12 +196,6 @@ mod python; pub mod type_object; pub mod types; -/// Internal utilities exposed for rust-numpy -#[doc(hidden)] -pub mod internal_utils { - pub use crate::gil::{ensure_gil, EnsureGIL}; -} - /// The proc macros, which are also part of the prelude. #[cfg(feature = "macros")] pub mod proc_macro { diff --git a/src/pycell.rs b/src/pycell.rs index ef1acb92372..d0d498d2d8e 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -730,7 +730,7 @@ impl fmt::Display for PyBorrowError { impl From for PyErr { fn from(other: PyBorrowError) -> Self { - PyRuntimeError::py_err(other.to_string()) + PyRuntimeError::new_err(other.to_string()) } } @@ -755,6 +755,6 @@ impl fmt::Display for PyBorrowMutError { impl From for PyErr { fn from(other: PyBorrowMutError) -> Self { - PyRuntimeError::py_err(other.to_string()) + PyRuntimeError::new_err(other.to_string()) } } diff --git a/src/pyclass.rs b/src/pyclass.rs index a4c10876493..52cc3910ca1 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -218,7 +218,7 @@ where if ffi::PyType_Ready(type_object) == 0 { Ok(()) } else { - PyErr::fetch(py).into() + Err(PyErr::fetch(py)) } } } diff --git a/src/python.rs b/src/python.rs index 901a426bcf0..fd736e99812 100644 --- a/src/python.rs +++ b/src/python.rs @@ -126,7 +126,7 @@ impl<'p> Python<'p> { /// .collect(); /// let mut sum = 0; /// for t in threads { - /// sum += t.join().map_err(|_| PyErr::new::(()))?; + /// sum += t.join().map_err(|_| PyRuntimeError::new_err(()))?; /// } /// Ok(sum) /// }) diff --git a/src/types/any.rs b/src/types/any.rs index aec69743427..cd973b0011d 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -163,7 +163,7 @@ impl PyAny { } else if do_compare(other, ffi::Py_GT)? { Ok(Ordering::Greater) } else { - Err(PyTypeError::py_err( + Err(PyTypeError::new_err( "PyAny::compare(): All comparisons returned false", )) } diff --git a/src/types/bytearray.rs b/src/types/bytearray.rs index 095c7564c99..ae7333825d6 100644 --- a/src/types/bytearray.rs +++ b/src/types/bytearray.rs @@ -295,7 +295,7 @@ mod test { let gil = Python::acquire_gil(); let py = gil.python(); let py_bytearray_result = PyByteArray::new_with(py, 10, |_b: &mut [u8]| { - Err(PyValueError::py_err("Hello Crustaceans!")) + Err(PyValueError::new_err("Hello Crustaceans!")) }); assert!(py_bytearray_result.is_err()); assert!(py_bytearray_result diff --git a/src/types/bytes.rs b/src/types/bytes.rs index 135b38cc025..ab5b13290a0 100644 --- a/src/types/bytes.rs +++ b/src/types/bytes.rs @@ -159,7 +159,7 @@ mod test { let gil = Python::acquire_gil(); let py = gil.python(); let py_bytes_result = PyBytes::new_with(py, 10, |_b: &mut [u8]| { - Err(PyValueError::py_err("Hello Crustaceans!")) + Err(PyValueError::new_err("Hello Crustaceans!")) }); assert!(py_bytes_result.is_err()); assert!(py_bytes_result diff --git a/src/types/num.rs b/src/types/num.rs index 14ab4b33ce4..09944d84486 100644 --- a/src/types/num.rs +++ b/src/types/num.rs @@ -40,7 +40,7 @@ macro_rules! int_fits_larger_int { fn extract(obj: &'source PyAny) -> PyResult { let val: $larger_type = obj.extract()?; <$rust_type>::try_from(val) - .map_err(|e| exceptions::PyOverflowError::py_err(e.to_string())) + .map_err(|e| exceptions::PyOverflowError::new_err(e.to_string())) } } }; @@ -86,7 +86,7 @@ macro_rules! int_fits_c_long { } }?; <$rust_type>::try_from(val) - .map_err(|e| exceptions::PyOverflowError::py_err(e.to_string())) + .map_err(|e| exceptions::PyOverflowError::new_err(e.to_string())) } } }; diff --git a/src/types/sequence.rs b/src/types/sequence.rs index 1b84352a5b0..67818de169a 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -361,7 +361,7 @@ where { let seq = ::try_from(obj)?; if seq.len()? as usize != slice.len() { - return Err(exceptions::PyBufferError::py_err( + return Err(exceptions::PyBufferError::new_err( "Slice length does not match buffer length.", )); } diff --git a/src/types/tuple.rs b/src/types/tuple.rs index e4c1d8d1c8c..77e7c903912 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -139,7 +139,7 @@ fn wrong_tuple_length(t: &PyTuple, expected_length: usize) -> PyErr { expected_length, t.len() ); - exceptions::PyValueError::py_err(msg) + exceptions::PyValueError::new_err(msg) } macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+} => { diff --git a/tests/test_buffer_protocol.rs b/tests/test_buffer_protocol.rs index c7343e19331..aca7ef1ca4e 100644 --- a/tests/test_buffer_protocol.rs +++ b/tests/test_buffer_protocol.rs @@ -21,11 +21,11 @@ struct TestBufferClass { impl PyBufferProtocol for TestBufferClass { fn bf_getbuffer(slf: PyRefMut, view: *mut ffi::Py_buffer, flags: c_int) -> PyResult<()> { if view.is_null() { - return Err(PyBufferError::py_err("View is null")); + return Err(PyBufferError::new_err("View is null")); } if (flags & ffi::PyBUF_WRITABLE) == ffi::PyBUF_WRITABLE { - return Err(PyBufferError::py_err("Object is not writable")); + return Err(PyBufferError::new_err("Object is not writable")); } unsafe { diff --git a/tests/test_dunder.rs b/tests/test_dunder.rs index 5cc56f61575..2633aff43ef 100644 --- a/tests/test_dunder.rs +++ b/tests/test_dunder.rs @@ -170,7 +170,7 @@ impl PySequenceProtocol for Sequence { if let Some(s) = self.fields.get(idx) { Ok(s.clone()) } else { - Err(PyErr::new::(())) + Err(PyIndexError::new_err(())) } } @@ -180,7 +180,7 @@ impl PySequenceProtocol for Sequence { *elem = value; Ok(()) } else { - Err(PyErr::new::(())) + Err(PyIndexError::new_err(())) } } } @@ -436,7 +436,7 @@ impl<'p> PyMappingProtocol<'p> for Test { return Ok("int"); } } - Err(PyErr::new::("error")) + Err(PyValueError::new_err("error")) } } diff --git a/tests/test_exceptions.rs b/tests/test_exceptions.rs index 3726dfb7e39..2c71669a2f8 100644 --- a/tests/test_exceptions.rs +++ b/tests/test_exceptions.rs @@ -46,7 +46,7 @@ impl fmt::Display for CustomError { impl std::convert::From for PyErr { fn from(err: CustomError) -> PyErr { - exceptions::PyOSError::py_err(err.to_string()) + exceptions::PyOSError::new_err(err.to_string()) } } diff --git a/tests/test_frompyobject.rs b/tests/test_frompyobject.rs index 0fa937cc853..98f2d2b9908 100644 --- a/tests/test_frompyobject.rs +++ b/tests/test_frompyobject.rs @@ -1,7 +1,7 @@ use pyo3::exceptions::PyValueError; use pyo3::prelude::*; use pyo3::types::{PyDict, PyString, PyTuple}; -use pyo3::{PyErrValue, PyMappingProtocol}; +use pyo3::PyMappingProtocol; #[macro_use] mod common; @@ -30,7 +30,7 @@ impl PyMappingProtocol for PyA { if key == "t" { Ok("bar".into()) } else { - Err(PyValueError::py_err("Failed")) + Err(PyValueError::new_err("Failed")) } } } @@ -277,7 +277,7 @@ fn test_enum() { } } -#[derive(FromPyObject)] +#[derive(Debug, FromPyObject)] pub enum Bar { #[pyo3(annotation = "str")] A(String), @@ -294,15 +294,8 @@ fn test_err_rename() { let dict = PyDict::new(py); let f = Bar::extract(dict.as_ref()); assert!(f.is_err()); - match f { - Ok(_) => {} - Err(e) => match e.pvalue { - PyErrValue::ToObject(to) => { - let o = to.to_object(py); - let s = String::extract(o.as_ref(py)).expect("Err val is not a string"); - assert_eq!(s, "Can't convert {} (dict) to Union[str, uint, int]") - } - _ => panic!("Expected PyErrValue::ToObject"), - }, - } + assert_eq!( + f.unwrap_err().to_string(), + "TypeError: Can't convert {} (dict) to Union[str, uint, int]" + ); } diff --git a/tests/test_mapping.rs b/tests/test_mapping.rs index 35369e01b4e..2e5f42df130 100644 --- a/tests/test_mapping.rs +++ b/tests/test_mapping.rs @@ -40,7 +40,7 @@ impl PyMappingProtocol for Mapping { self.index .get(&query) .copied() - .ok_or_else(|| PyKeyError::py_err("unknown key")) + .ok_or_else(|| PyKeyError::new_err("unknown key")) } fn __setitem__(&mut self, key: String, value: usize) { @@ -49,7 +49,7 @@ impl PyMappingProtocol for Mapping { fn __delitem__(&mut self, key: String) -> PyResult<()> { if self.index.remove(&key).is_none() { - PyKeyError::py_err("unknown key").into() + Err(PyKeyError::new_err("unknown key")) } else { Ok(()) } diff --git a/tests/test_sequence.rs b/tests/test_sequence.rs index d46edfb3b45..0a4c4afd1f8 100644 --- a/tests/test_sequence.rs +++ b/tests/test_sequence.rs @@ -41,7 +41,7 @@ impl PySequenceProtocol for ByteSequence { self.elements .get(idx as usize) .copied() - .ok_or_else(|| PyIndexError::py_err("list index out of range")) + .ok_or_else(|| PyIndexError::new_err("list index out of range")) } fn __setitem__(&mut self, idx: isize, value: u8) { @@ -53,7 +53,7 @@ impl PySequenceProtocol for ByteSequence { self.elements.remove(idx as usize); Ok(()) } else { - Err(PyIndexError::py_err("list index out of range")) + Err(PyIndexError::new_err("list index out of range")) } } @@ -78,7 +78,7 @@ impl PySequenceProtocol for ByteSequence { } Ok(Self { elements }) } else { - Err(PyValueError::py_err("invalid repeat count")) + Err(PyValueError::new_err("invalid repeat count")) } } } @@ -242,7 +242,7 @@ impl PySequenceProtocol for OptionList { fn __getitem__(&self, idx: isize) -> PyResult> { match self.items.get(idx as usize) { Some(x) => Ok(*x), - None => Err(PyIndexError::py_err("Index out of bounds")), + None => Err(PyIndexError::new_err("Index out of bounds")), } } } diff --git a/tests/test_various.rs b/tests/test_various.rs index 87270c39186..49923bf299a 100644 --- a/tests/test_various.rs +++ b/tests/test_various.rs @@ -191,7 +191,7 @@ impl fmt::Display for MyError { /// Important for the automatic conversion to `PyErr`. impl From for PyErr { fn from(err: MyError) -> pyo3::PyErr { - pyo3::exceptions::PyOSError::py_err(err.to_string()) + pyo3::exceptions::PyOSError::new_err(err.to_string()) } } From c519b4de765afd4c91118ba5201f355395a919d7 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Thu, 10 Sep 2020 21:33:17 +0100 Subject: [PATCH 5/7] Fix build for new apis --- src/types/function.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/types/function.rs b/src/types/function.rs index 73aa04b7426..d215d6c4a43 100644 --- a/src/types/function.rs +++ b/src/types/function.rs @@ -42,9 +42,10 @@ impl PyCFunction { ) -> PyResult<&'a PyCFunction> { let (py, module) = py_or_module.into_py_and_maybe_module(); let doc: &'static CStr = CStr::from_bytes_with_nul(doc.as_bytes()) - .map_err(|_| PyValueError::py_err("docstring must end with NULL byte."))?; - let name = CString::new(name.as_bytes()) - .map_err(|_| PyValueError::py_err("Function name cannot contain contain NULL byte."))?; + .map_err(|_| PyValueError::new_err("docstring must end with NULL byte."))?; + let name = CString::new(name.as_bytes()).map_err(|_| { + PyValueError::new_err("Function name cannot contain contain NULL byte.") + })?; let def = match fun { PyMethodType::PyCFunction(fun) => ffi::PyMethodDef { ml_name: name.into_raw() as _, @@ -59,7 +60,7 @@ impl PyCFunction { ml_doc: doc.as_ptr() as _, }, _ => { - return Err(PyValueError::py_err( + return Err(PyValueError::new_err( "Only PyCFunction and PyCFunctionWithKeywords are valid.", )) } From 4d5c208c9ecfa28e578c2bf16d65aa61e57fb320 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Thu, 10 Sep 2020 17:05:24 -0400 Subject: [PATCH 6/7] fixes --- src/err/mod.rs | 7 ++++++- src/pyclass.rs | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/err/mod.rs b/src/err/mod.rs index 724ce0caecf..384fb259789 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -494,7 +494,12 @@ impl<'a> std::fmt::Display for PyDowncastError<'a> { self.from .repr() .map(|s| s.to_string_lossy()) - .unwrap_or_else(|_| self.from.get_type().name()), + .or_else(|_| self + .from + .get_type() + .name() + .map_err(|_| std::fmt::Error) + .map(|s| s.into()))?, self.to ) } diff --git a/src/pyclass.rs b/src/pyclass.rs index 97c2a6b73d8..85c1c60a62c 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -214,7 +214,7 @@ where let type_object = unsafe { ffi::PyType_FromSpec(&mut spec) }; if type_object.is_null() { - PyErr::fetch(py).into() + Err(PyErr::fetch(py)) } else { tp_init_additional::(type_object as _); Ok(type_object as _) @@ -275,7 +275,7 @@ fn fallback_new() -> Option { _kwds: *mut ffi::PyObject, ) -> *mut ffi::PyObject { crate::callback_body!(py, { - Err::<(), _>(crate::exceptions::PyTypeError::py_err( + Err::<(), _>(crate::exceptions::PyTypeError::new_err( "No constructor defined", )) }) From 3cb0b112d2e9116607fa025983f1029748726005 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Sat, 12 Sep 2020 09:47:42 -0400 Subject: [PATCH 7/7] Update src/err/mod.rs Co-authored-by: Yuji Kanagawa --- src/err/mod.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/err/mod.rs b/src/err/mod.rs index 384fb259789..38af8ba989f 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -494,12 +494,8 @@ impl<'a> std::fmt::Display for PyDowncastError<'a> { self.from .repr() .map(|s| s.to_string_lossy()) - .or_else(|_| self - .from - .get_type() - .name() - .map_err(|_| std::fmt::Error) - .map(|s| s.into()))?, + .or_else(|_| self.from.get_type().name().map(|s| s.into())) + .map_err(|_| std::fmt::Error)?, self.to ) }