luxolus/pyo3
2
0
Fork 0
Code Issues Pull requests Packages Projects Releases Wiki Activity

Don't use intocallback in method macros (#2664)

Browse source 
* Don't use intocallback in method macros

Co-authored-by: mejrs <>
This commit is contained in:
Bruno Kolenbrander 2022-10-16 11:35:58 +02:00 committed by GitHub
parent 125af9b7de
commit 4a04603c2c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 71 additions and 107 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
Cargo.toml
View file

@ -44,7 +44,8 @@ serde = { version = "1.0", optional = true }
assert_approx_eq = "1.1.0"
chrono = { version = "0.4" }
criterion = "0.3.5"
trybuild = "1.0.49"
# Required for "and $N others" normalization
trybuild = ">=1.0.70"
rustversion = "1.0"
# 1.0.0 requires Rust 1.50
proptest = { version = "0.10.1", default-features = false, features = ["std"] }

1
newsfragments/2664.changed.md Normal file
View file

@ -0,0 +1 @@
PyO3's macros now emit a much nicer error message if function return values don't implement the required trait(s).

11
pyo3-macros-backend/src/method.rs
View file

@ -468,17 +468,12 @@ impl<'a> FnSpec<'a> {
quote!(#func_name)
};
// The method call is necessary to generate a decent error message.
let rust_call = |args: Vec<TokenStream>| {
quote! {
let mut ret = #rust_name(#self_arg #(#args),*);
if false {
use _pyo3::impl_::ghost::IntoPyResult;
ret.assert_into_py_result();
}
_pyo3::callback::convert(#py, ret)
let owned = _pyo3::impl_::pymethods::OkWrap::wrap(ret, #py);
owned.map(|obj| _pyo3::conversion::IntoPyPointer::into_ptr(obj))
.map_err(::core::convert::Into::into)
}
};

7
pyo3-macros-backend/src/pymethod.rs
View file

@ -389,11 +389,8 @@ fn impl_py_class_attribute(cls: &syn::Type, spec: &FnSpec<'_>) -> syn::Result<Me
fn #wrapper_ident(py: _pyo3::Python<'_>) -> _pyo3::PyResult<_pyo3::PyObject> {
#deprecations
let mut ret = #fncall;
if false {
use _pyo3::impl_::ghost::IntoPyResult;
ret.assert_into_py_result();
}
_pyo3::callback::convert(py, ret)
let owned = _pyo3::impl_::pymethods::OkWrap::wrap(ret, py);
owned.map_err(::core::convert::Into::into)
}
};

1
src/impl_.rs
View file

@ -10,7 +10,6 @@ pub mod deprecations;
pub mod extract_argument;
pub mod freelist;
pub mod frompyobject;
pub mod ghost;
pub(crate) mod not_send;
pub mod panic;
pub mod pycell;

18
src/impl_/ghost.rs
View file

@ -1,18 +0,0 @@
/// If it does nothing, was it ever really there? 👻
///
/// This is code that is just type checked to e.g. create better compile errors,
/// but that never affects anything at runtime,
use crate::{IntoPy, PyErr, PyObject};
pub trait IntoPyResult<T> {
fn assert_into_py_result(&mut self) {}
}
impl<T> IntoPyResult<T> for T where T: IntoPy<PyObject> {}
impl<T, E> IntoPyResult<T> for Result<T, E>
where
T: IntoPy<PyObject>,
E: Into<PyErr>,
{
}

28
src/impl_/pymethods.rs
View file

@ -1,5 +1,5 @@
use crate::internal_tricks::{extract_cstr_or_leak_cstring, NulByteInString};
use crate::{ffi, PyAny, PyObject, PyResult, PyTraverseError, Python};
use crate::{ffi, IntoPy, Py, PyAny, PyErr, PyObject, PyResult, PyTraverseError, Python};
use std::ffi::CStr;
use std::fmt;
use std::os::raw::c_int;
@ -262,3 +262,29 @@ pub fn unwrap_traverse_result(result: Result<(), PyTraverseError>) -> c_int {
Err(PyTraverseError(value)) => value,
}
}
// The macros need to Ok-wrap the output of user defined functions; i.e. if they're not a result, make them into one.
pub trait OkWrap<T> {
type Error;
fn wrap(self, py: Python<'_>) -> Result<Py<PyAny>, Self::Error>;
}
impl<T> OkWrap<T> for T
where
T: IntoPy<PyObject>,
{
type Error = PyErr;
fn wrap(self, py: Python<'_>) -> PyResult<Py<PyAny>> {
Ok(self.into_py(py))
}
}
impl<T, E> OkWrap<T> for Result<T, E>
where
T: IntoPy<PyObject>,
{
type Error = E;
fn wrap(self, py: Python<'_>) -> Result<Py<PyAny>, Self::Error> {
self.map(|o| o.into_py(py))
}
}

8
src/pyclass.rs
View file

@ -341,6 +341,10 @@ impl PyTypeBuilder {
module_name: Option<&'static str>,
basicsize: usize,
) -> PyResult<*mut ffi::PyTypeObject> {
// `c_ulong` and `c_uint` have the same size
// on some platforms (like windows)
#![allow(clippy::useless_conversion)]
self.finalize_methods_and_properties();
if !self.has_new {
@ -376,9 +380,7 @@ impl PyTypeBuilder {
name: py_class_qualified_name(module_name, name)?,
basicsize: basicsize as c_int,
itemsize: 0,
// `c_ulong` and `c_uint` have the same size
// on some platforms (like windows)
#[allow(clippy::useless_conversion)]
flags: (ffi::Py_TPFLAGS_DEFAULT | self.class_flags)
.try_into()
.unwrap(),

38
tests/ui/invalid_result_conversion.stderr
View file

@ -1,30 +1,18 @@
error[E0599]: no method named `assert_into_py_result` found for enum `Result` in the current scope
error[E0277]: the trait bound `PyErr: From<MyError>` is not satisfied
--> tests/ui/invalid_result_conversion.rs:21:1
|
21 | #[pyfunction]
| ^^^^^^^^^^^^^ method not found in `Result<(), MyError>`
| ^^^^^^^^^^^^^ the trait `From<MyError>` is not implemented for `PyErr`
|
note: the method `assert_into_py_result` exists on the type `()`
--> src/impl_/ghost.rs
|
| fn assert_into_py_result(&mut self) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: the following other types implement trait `From<T>`:
<PyErr as From<&CancelledError>>
<PyErr as From<&IncompleteReadError>>
<PyErr as From<&InvalidStateError>>
<PyErr as From<&LimitOverrunError>>
<PyErr as From<&PanicException>>
<PyErr as From<&PyArithmeticError>>
<PyErr as From<&PyAssertionError>>
<PyErr as From<&PyAttributeError>>
and $N others
= note: required because of the requirements on the impl of `Into<PyErr>` for `MyError`
= note: this error originates in the attribute macro `pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)
help: use the `?` operator to extract the `()` value, propagating a `Result::Err` value to the caller
|
21 | #[pyfunction]?
| +
error[E0277]: the trait bound `Result<(), MyError>: IntoPyCallbackOutput<_>` is not satisfied
--> tests/ui/invalid_result_conversion.rs:21:1
|
21 | #[pyfunction]
| ^^^^^^^^^^^^^ the trait `IntoPyCallbackOutput<_>` is not implemented for `Result<(), MyError>`
|
= help: the trait `IntoPyCallbackOutput<U>` is implemented for `Result<T, E>`
note: required by a bound in `pyo3::callback::convert`
--> src/callback.rs
|
| T: IntoPyCallbackOutput<U>,
| ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `pyo3::callback::convert`
= note: this error originates in the attribute macro `pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)

63
tests/ui/missing_intopy.stderr
View file

@ -1,45 +1,18 @@
error[E0599]: the method `assert_into_py_result` exists for struct `Blah`, but its trait bounds were not satisfied
--> tests/ui/missing_intopy.rs:3:1
|
1 | struct Blah;
| -----------
| |
| method `assert_into_py_result` not found for this struct
| doesn't satisfy `Blah: IntoPy<Py<PyAny>>`
| doesn't satisfy `Blah: IntoPyResult<Blah>`
2 |
3 | #[pyo3::pyfunction]
| ^^^^^^^^^^^^^^^^^^^ method cannot be called on `Blah` due to unsatisfied trait bounds
|
= note: the following trait bounds were not satisfied:
`Blah: IntoPy<Py<PyAny>>`
which is required by `Blah: IntoPyResult<Blah>`
note: the following trait must be implemented
--> src/conversion.rs
|
| pub trait IntoPy<T>: Sized {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: this error originates in the attribute macro `pyo3::pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0277]: the trait bound `Blah: IntoPyCallbackOutput<_>` is not satisfied
--> tests/ui/missing_intopy.rs:3:1
|
3 | #[pyo3::pyfunction]
| ^^^^^^^^^^^^^^^^^^^ the trait `IntoPyCallbackOutput<_>` is not implemented for `Blah`
|
= help: the following other types implement trait `IntoPyCallbackOutput<Target>`:
<() as IntoPyCallbackOutput<()>>
<() as IntoPyCallbackOutput<i32>>
<*mut PyObject as IntoPyCallbackOutput<*mut PyObject>>
<HashCallbackOutput as IntoPyCallbackOutput<isize>>
<IterANextOutput<Py<PyAny>, Py<PyAny>> as IntoPyCallbackOutput<*mut PyObject>>
<IterANextOutput<T, U> as IntoPyCallbackOutput<IterANextOutput<Py<PyAny>, Py<PyAny>>>>
<IterNextOutput<Py<PyAny>, Py<PyAny>> as IntoPyCallbackOutput<*mut PyObject>>
<IterNextOutput<T, U> as IntoPyCallbackOutput<IterNextOutput<Py<PyAny>, Py<PyAny>>>>
and 7 others
note: required by a bound in `pyo3::callback::convert`
--> src/callback.rs
|
| T: IntoPyCallbackOutput<U>,
| ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `pyo3::callback::convert`
= note: this error originates in the attribute macro `pyo3::pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0277]: the trait bound `Blah: IntoPy<Py<PyAny>>` is not satisfied
--> tests/ui/missing_intopy.rs:3:1
|
3 | #[pyo3::pyfunction]
| ^^^^^^^^^^^^^^^^^^^ the trait `IntoPy<Py<PyAny>>` is not implemented for `Blah`
|
= help: the following other types implement trait `IntoPy<T>`:
<&'a OsString as IntoPy<Py<PyAny>>>
<&'a Path as IntoPy<Py<PyAny>>>
<&'a PathBuf as IntoPy<Py<PyAny>>>
<&'a PyErr as IntoPy<Py<PyAny>>>
<&'a String as IntoPy<Py<PyAny>>>
<&'a [u8] as IntoPy<Py<PyAny>>>
<&'a str as IntoPy<Py<PyAny>>>
<&'a str as IntoPy<Py<PyString>>>
and $N others
= note: required because of the requirements on the impl of `OkWrap<Blah>` for `Blah`
= note: this error originates in the attribute macro `pyo3::pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)