From 64a2456d5e4dda1c83296c6d5725cc3f7885ef9e Mon Sep 17 00:00:00 2001 From: mejrs Date: Wed, 24 Nov 2021 11:25:41 +0100 Subject: [PATCH 1/3] Allow user defined exceptions to have docstrings --- src/err/mod.rs | 85 +++++++++++++++++++++++++++++++++------ src/exceptions.rs | 90 ++++++++++++++++++++++++++++++++---------- src/internal_tricks.rs | 2 +- src/panic.rs | 10 ++--- 4 files changed, 149 insertions(+), 38 deletions(-) diff --git a/src/err/mod.rs b/src/err/mod.rs index db8e2571fde..4f31a96e334 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -11,7 +11,6 @@ use crate::{AsPyPointer, IntoPy, Py, PyAny, PyObject, Python, ToBorrowedObject, use std::borrow::Cow; use std::cell::UnsafeCell; use std::ffi::CString; -use std::os::raw::c_char; use std::os::raw::c_int; use std::ptr::NonNull; @@ -306,13 +305,20 @@ impl PyErr { } } - /// Creates a new exception type with the given name, which must be of the form - /// `.`, as required by `PyErr_NewException`. + /// Creates a new exception type with the given name. /// - /// `base` can be an existing exception type to subclass, or a tuple of classes - /// `dict` specifies an optional dictionary of class variables and methods + /// - `base` can be an existing exception type to subclass, or a tuple of classes. + /// - `dict` specifies an optional dictionary of class variables and methods. + /// + /// For a version of this function that also takes a docstring, see [`PyErr::new_type_with_doc`]. + /// + /// # Panics + /// + /// This function will panic if: + /// - `name` is not of the form `.`. + /// - `name` cannot be converted to [`CString`]s. pub fn new_type<'p>( - _: Python<'p>, + py: Python<'p>, name: &str, base: Option<&PyType>, dict: Option, @@ -327,15 +333,70 @@ impl PyErr { Some(obj) => obj.as_ptr(), }; - unsafe { - let null_terminated_name = - CString::new(name).expect("Failed to initialize nul terminated exception name"); + let null_terminated_name = + CString::new(name).expect("Failed to initialize nul terminated exception name"); + + let ptr = unsafe { + ffi::PyErr_NewException(null_terminated_name.as_ptr(), base, dict) + as *mut ffi::PyTypeObject + }; + match NonNull::new(ptr) { + Some(not_null) => not_null, + None => panic!("{}", PyErr::fetch(py)), + } + } - NonNull::new_unchecked(ffi::PyErr_NewException( - null_terminated_name.as_ptr() as *mut c_char, + /// Creates a new exception type with the given name and docstring. + /// + /// - `base` can be an existing exception type to subclass, or a tuple of classes. + /// - `dict` specifies an optional dictionary of class variables and methods. + /// - `doc` will be the docstring seen by python users. + /// + /// # Panics + /// + /// This function will panic if: + /// - `name` is not of the form `.`. + /// - `name` or `doc` cannot be converted to [`CString`]s. + pub fn new_type_with_doc<'p>( + py: Python<'p>, + name: &str, + doc: Option<&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(), + }; + + let null_terminated_name = + CString::new(name).expect("Failed to initialize nul terminated exception name"); + + let null_terminated_doc = + doc.map(|d| CString::new(d).expect("Failed to initialize nul terminated docstring")); + + let null_terminated_doc_ptr = match null_terminated_doc.as_ref() { + Some(c) => c.as_ptr(), + None => std::ptr::null(), + }; + + let ptr = unsafe { + ffi::PyErr_NewExceptionWithDoc( + null_terminated_name.as_ptr(), + null_terminated_doc_ptr, base, dict, - ) as *mut ffi::PyTypeObject) + ) as *mut ffi::PyTypeObject + }; + + match NonNull::new(ptr) { + Some(not_null) => not_null, + None => panic!("{}", PyErr::fetch(py)), } } diff --git a/src/exceptions.rs b/src/exceptions.rs index 004e19f4f9f..7af72ae62c0 100644 --- a/src/exceptions.rs +++ b/src/exceptions.rs @@ -129,33 +129,67 @@ macro_rules! import_exception { /// /// # Syntax /// -/// ```create_exception!(module, MyError, BaseException)``` -/// /// * `module` is the name of the containing module. -/// * `MyError` is the name of the new exception type. -/// * `BaseException` is the superclass of `MyError`, usually `pyo3::exceptions::PyException`. +/// * `name` is the name of the new exception type. +/// * `base` is the superclass of `MyError`, usually [`PyException`]. +/// * `doc` (optional) is the docstring visible to users (with `.__doc__` and `help()`) and +/// accompanies your error type in your crate's documentation. /// /// # Examples +/// /// ``` /// use pyo3::prelude::*; /// use pyo3::create_exception; -/// use pyo3::types::IntoPyDict; /// use pyo3::exceptions::PyException; /// -/// create_exception!(mymodule, CustomError, PyException); +/// create_exception!(my_module, MyError, PyException, "Some description."); /// -/// Python::with_gil(|py| { -/// let error_type = py.get_type::(); -/// let ctx = [("CustomError", error_type)].into_py_dict(py); -/// let type_description: String = py -/// .eval("str(CustomError)", None, Some(&ctx)) -/// .unwrap() -/// .extract() -/// .unwrap(); -/// assert_eq!(type_description, ""); -/// pyo3::py_run!(py, *ctx, "assert CustomError('oops').args == ('oops',)"); -/// }); +/// #[pyfunction] +/// fn raise_myerror() -> PyResult<()>{ +/// let err = MyError::new_err("Some error happened."); +/// Err(err) +/// } +/// +/// #[pymodule] +/// fn my_module(py: Python, m: &PyModule) -> PyResult<()> { +/// m.add("MyError", py.get_type::())?; +/// m.add_function(wrap_pyfunction!(raise_myerror, py)?)?; +/// Ok(()) +/// } +/// # fn main() -> PyResult<()> { +/// # Python::with_gil(|py| -> PyResult<()> { +/// # let fun = wrap_pyfunction!(raise_myerror, py)?; +/// # let globals = pyo3::types::PyDict::new(py); +/// # globals.set_item("MyError", py.get_type::())?; +/// # globals.set_item("raise_myerror", fun)?; +/// # +/// # py.run( +/// # "try: +/// # raise_myerror() +/// # except MyError as e: +/// # assert e.__doc__ == 'Some description.' +/// # assert str(e) == 'Some error happened.'", +/// # Some(globals), +/// # None, +/// # )?; +/// # +/// # Ok(()) +/// # }) +/// # } +/// ``` +/// +/// Python code can handle this exception like any other exception: +/// +/// ```python +/// from my_module import MyError, raise_myerror +/// +/// try: +/// raise_myerror() +/// except MyError as e: +/// assert e.__doc__ == 'Some description.' +/// assert str(e) == 'Some error happened.' /// ``` +/// #[macro_export] macro_rules! create_exception { ($module: ident, $name: ident, $base: ty) => { @@ -165,7 +199,22 @@ macro_rules! create_exception { $crate::impl_exception_boilerplate!($name); - $crate::create_exception_type_object!($module, $name, $base); + $crate::create_exception_type_object!($module, $name, $base, ::std::option::Option::None); + }; + ($module: ident, $name: ident, $base: ty, $doc: expr) => { + #[repr(transparent)] + #[allow(non_camel_case_types)] // E.g. `socket.herror` + #[doc = $doc] + pub struct $name($crate::PyAny); + + $crate::impl_exception_boilerplate!($name); + + $crate::create_exception_type_object!( + $module, + $name, + $base, + ::std::option::Option::Some($doc) + ); }; } @@ -174,7 +223,7 @@ macro_rules! create_exception { #[doc(hidden)] #[macro_export] macro_rules! create_exception_type_object { - ($module: ident, $name: ident, $base: ty) => { + ($module: ident, $name: ident, $base: ty, $doc: expr) => { $crate::pyobject_native_type_core!( $name, *$name::type_object_raw($crate::Python::assume_gil_acquired()), @@ -192,9 +241,10 @@ macro_rules! create_exception_type_object { .get_or_init(py, || unsafe { $crate::Py::from_owned_ptr( py, - $crate::PyErr::new_type( + $crate::PyErr::new_type_with_doc( py, concat!(stringify!($module), ".", stringify!($name)), + $doc, ::std::option::Option::Some(py.get_type::<$base>()), ::std::option::Option::None, ) diff --git a/src/internal_tricks.rs b/src/internal_tricks.rs index 4d443ed08db..307f1bfc9e3 100644 --- a/src/internal_tricks.rs +++ b/src/internal_tricks.rs @@ -35,7 +35,7 @@ macro_rules! pyo3_exception { $crate::impl_exception_boilerplate!($name); - $crate::create_exception_type_object!(pyo3_runtime, $name, $base); + $crate::create_exception_type_object!(pyo3_runtime, $name, $base, Some($doc)); }; } diff --git a/src/panic.rs b/src/panic.rs index cf1dd3701e3..0fe6c0bf43f 100644 --- a/src/panic.rs +++ b/src/panic.rs @@ -5,12 +5,12 @@ use std::any::Any; pyo3_exception!( " - The exception raised when Rust code called from Python panics. +The exception raised when Rust code called from Python panics. - Like SystemExit, this exception is derived from BaseException so that - it will typically propagate all the way through the stack and cause the - Python interpreter to exit. - ", +Like SystemExit, this exception is derived from BaseException so that +it will typically propagate all the way through the stack and cause the +Python interpreter to exit. +", PanicException, PyBaseException ); From 0ee13c1c1b6a34d0e1d80228c484edb6837ec47e Mon Sep 17 00:00:00 2001 From: mejrs Date: Sat, 4 Dec 2021 12:52:19 +0100 Subject: [PATCH 2/3] Make new_type return Py instead --- src/err/mod.rs | 62 ++++++-------------------------- src/exceptions.rs | 90 +++++++++++++++++++++++++++++++++++++---------- 2 files changed, 82 insertions(+), 70 deletions(-) diff --git a/src/err/mod.rs b/src/err/mod.rs index 4f31a96e334..cc44611926f 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -12,7 +12,6 @@ use std::borrow::Cow; use std::cell::UnsafeCell; use std::ffi::CString; use std::os::raw::c_int; -use std::ptr::NonNull; mod err_state; mod impls; @@ -305,65 +304,27 @@ impl PyErr { } } - /// Creates a new exception type with the given name. + /// Creates a new exception type with the given name and docstring. /// /// - `base` can be an existing exception type to subclass, or a tuple of classes. /// - `dict` specifies an optional dictionary of class variables and methods. + /// - `doc` will be the docstring seen by python users. /// - /// For a version of this function that also takes a docstring, see [`PyErr::new_type_with_doc`]. /// - /// # Panics + /// # Errors /// - /// This function will panic if: - /// - `name` is not of the form `.`. - /// - `name` cannot be converted to [`CString`]s. - pub fn new_type<'p>( - py: 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(), - }; - - let null_terminated_name = - CString::new(name).expect("Failed to initialize nul terminated exception name"); - - let ptr = unsafe { - ffi::PyErr_NewException(null_terminated_name.as_ptr(), base, dict) - as *mut ffi::PyTypeObject - }; - match NonNull::new(ptr) { - Some(not_null) => not_null, - None => panic!("{}", PyErr::fetch(py)), - } - } - - /// Creates a new exception type with the given name and docstring. - /// - /// - `base` can be an existing exception type to subclass, or a tuple of classes. - /// - `dict` specifies an optional dictionary of class variables and methods. - /// - `doc` will be the docstring seen by python users. + /// This function returns an error if `name` is not of the form `.`. /// /// # Panics /// - /// This function will panic if: - /// - `name` is not of the form `.`. - /// - `name` or `doc` cannot be converted to [`CString`]s. - pub fn new_type_with_doc<'p>( - py: Python<'p>, + /// This function will panic if `name` or `doc` cannot be converted to [`CString`]s. + pub fn new_type( + py: Python, name: &str, doc: Option<&str>, base: Option<&PyType>, dict: Option, - ) -> NonNull { + ) -> PyResult> { let base: *mut ffi::PyObject = match base { None => std::ptr::null_mut(), Some(obj) => obj.as_ptr(), @@ -391,13 +352,10 @@ impl PyErr { null_terminated_doc_ptr, base, dict, - ) as *mut ffi::PyTypeObject + ) }; - match NonNull::new(ptr) { - Some(not_null) => not_null, - None => panic!("{}", PyErr::fetch(py)), - } + unsafe { Py::from_owned_ptr_or_err(py, ptr) } } /// Prints a standard traceback to `sys.stderr`. diff --git a/src/exceptions.rs b/src/exceptions.rs index 7af72ae62c0..fcf5860f593 100644 --- a/src/exceptions.rs +++ b/src/exceptions.rs @@ -131,7 +131,7 @@ macro_rules! import_exception { /// /// * `module` is the name of the containing module. /// * `name` is the name of the new exception type. -/// * `base` is the superclass of `MyError`, usually [`PyException`]. +/// * `base` is the base class of `MyError`, usually [`PyException`]. /// * `doc` (optional) is the docstring visible to users (with `.__doc__` and `help()`) and /// accompanies your error type in your crate's documentation. /// @@ -159,9 +159,9 @@ macro_rules! import_exception { /// # fn main() -> PyResult<()> { /// # Python::with_gil(|py| -> PyResult<()> { /// # let fun = wrap_pyfunction!(raise_myerror, py)?; -/// # let globals = pyo3::types::PyDict::new(py); -/// # globals.set_item("MyError", py.get_type::())?; -/// # globals.set_item("raise_myerror", fun)?; +/// # let locals = pyo3::types::PyDict::new(py); +/// # locals.set_item("MyError", py.get_type::())?; +/// # locals.set_item("raise_myerror", fun)?; /// # /// # py.run( /// # "try: @@ -169,8 +169,8 @@ macro_rules! import_exception { /// # except MyError as e: /// # assert e.__doc__ == 'Some description.' /// # assert str(e) == 'Some error happened.'", -/// # Some(globals), /// # None, +/// # Some(locals), /// # )?; /// # /// # Ok(()) @@ -238,20 +238,15 @@ macro_rules! create_exception_type_object { GILOnceCell::new(); TYPE_OBJECT - .get_or_init(py, || unsafe { - $crate::Py::from_owned_ptr( + .get_or_init(py, || + $crate::PyErr::new_type( py, - $crate::PyErr::new_type_with_doc( - py, - concat!(stringify!($module), ".", stringify!($name)), - $doc, - ::std::option::Option::Some(py.get_type::<$base>()), - ::std::option::Option::None, - ) - .as_ptr() as *mut $crate::ffi::PyObject, - ) - }) - .as_ptr() as *mut _ + concat!(stringify!($module), ".", stringify!($name)), + $doc, + ::std::option::Option::Some(py.get_type::<$base>()), + ::std::option::Option::None, + ).expect("Failed to initialize new exception type.") + ).as_ptr() as *mut $crate::ffi::PyTypeObject } } }; @@ -796,6 +791,65 @@ mod tests { Some(ctx), ) .unwrap(); + py.run("assert CustomError.__doc__ is None", None, Some(ctx)) + .unwrap(); + }); + } + + #[test] + fn custom_exception_doc() { + create_exception!(mymodule, CustomError, PyException, "Some docs"); + + Python::with_gil(|py| { + let error_type = py.get_type::(); + let ctx = [("CustomError", error_type)].into_py_dict(py); + let type_description: String = py + .eval("str(CustomError)", None, Some(ctx)) + .unwrap() + .extract() + .unwrap(); + assert_eq!(type_description, ""); + py.run( + "assert CustomError('oops').args == ('oops',)", + None, + Some(ctx), + ) + .unwrap(); + py.run("assert CustomError.__doc__ == 'Some docs'", None, Some(ctx)) + .unwrap(); + }); + } + + #[test] + fn custom_exception_doc_expr() { + create_exception!( + mymodule, + CustomError, + PyException, + concat!("Some", " more ", stringify!(docs)) + ); + + Python::with_gil(|py| { + let error_type = py.get_type::(); + let ctx = [("CustomError", error_type)].into_py_dict(py); + let type_description: String = py + .eval("str(CustomError)", None, Some(ctx)) + .unwrap() + .extract() + .unwrap(); + assert_eq!(type_description, ""); + py.run( + "assert CustomError('oops').args == ('oops',)", + None, + Some(ctx), + ) + .unwrap(); + py.run( + "assert CustomError.__doc__ == 'Some more docs'", + None, + Some(ctx), + ) + .unwrap(); }); } From bbe478db818ba09083de0aede148dfba1980243e Mon Sep 17 00:00:00 2001 From: mejrs Date: Tue, 14 Dec 2021 19:19:31 +0100 Subject: [PATCH 3/3] Update changelog --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de1f6233899..cd89e06b383 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `into_instance` -> `into_value` - Deprecate `PyType::is_instance`; it is inconsistent with other `is_instance` methods in PyO3. Instead of `typ.is_instance(obj)`, use `obj.is_instance(typ)`. [#2031](https://github.com/PyO3/pyo3/pull/2031) - Optional parameters of `#[pymethods]` and `#[pyfunction]`s cannot be followed by required parameters, i.e. `fn opt_first(a: Option, b: i32) {}` is not allowed, while `fn opt_last(a:i32, b: Option) {}` is. [#2041](https://github.com/PyO3/pyo3/pull/2041) - +- `PyErr::new_type` now takes an optional docstring and now returns `PyResult>` rather than a `ffi::PyTypeObject` pointer. +- The `create_exception!` macro can now take an optional docstring. This docstring, if supplied, is visible to users (with `.__doc__` and `help()`) and + accompanies your error type in your crate's documentation. + ### Removed - Remove all functionality deprecated in PyO3 0.14. [#2007](https://github.com/PyO3/pyo3/pull/2007)