Skip to content

Commit

Permalink
Use PyErr::from_value_bound instead of from_value
Browse files Browse the repository at this point in the history
  • Loading branch information
LilyFoote committed Feb 12, 2024
1 parent 00c6028 commit 6371ce1
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 42 deletions.
2 changes: 1 addition & 1 deletion src/coroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl Coroutine {
(Some(exc), Some(cb)) => cb.throw(exc.as_ref(py)),
(Some(exc), None) => {
self.close();
return Err(PyErr::from_value(exc.as_ref(py)));
return Err(PyErr::from_value_bound(exc.bind(py)));
}
(None, _) => {}
}
Expand Down
7 changes: 5 additions & 2 deletions src/err/err_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,11 @@ impl PyErrState {
}))
}

pub(crate) fn lazy_bound(ptype: Bound<'_, PyAny>, args: impl PyErrArguments + 'static) -> Self {
let ptype = ptype.into();
pub(crate) fn lazy_bound(
ptype: &Bound<'_, PyAny>,
args: impl PyErrArguments + 'static,
) -> Self {
let ptype = (*ptype).clone().into();
PyErrState::Lazy(Box::new(move |py| PyErrStateLazyFnOutput {
ptype,
pvalue: args.arguments(py),
Expand Down
37 changes: 19 additions & 18 deletions src/err/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,16 +183,15 @@ impl PyErr {
}

/// Deprecated form of [`PyErr::from_value_bound`].
//#[cfg_attr(
// not(feature = "gil-refs"),
// deprecated(
// since = "0.21.0",
// note = "`PyErr::from_value` will be replaced by `PyErr::from_value_bound` in a future PyO3 version"
// )
//)]
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "`PyErr::from_value` will be replaced by `PyErr::from_value_bound` in a future PyO3 version"
)
)]
pub fn from_value(obj: &PyAny) -> PyErr {
let py = obj.py();
PyErr::from_value_bound(obj.into_py(py).into_bound(py))
PyErr::from_value_bound(&obj.as_borrowed())
}

/// Creates a new PyErr.
Expand All @@ -212,23 +211,23 @@ impl PyErr {
///
/// Python::with_gil(|py| {
/// // Case #1: Exception object
/// let err = PyErr::from_value_bound(PyTypeError::new_err("some type error")
/// .value_bound(py).clone().into_any());
/// let err = PyErr::from_value_bound(&PyTypeError::new_err("some type error")
/// .value_bound(py).as_borrowed());
/// assert_eq!(err.to_string(), "TypeError: some type error");
///
/// // Case #2: Exception type
/// let err = PyErr::from_value_bound(PyTypeError::type_object_bound(py).into_any());
/// let err = PyErr::from_value_bound(&PyTypeError::type_object_bound(py).as_borrowed());
/// assert_eq!(err.to_string(), "TypeError: ");
///
/// // Case #3: Invalid exception value
/// let err = PyErr::from_value_bound(PyString::new_bound(py, "foo").into_any());
/// let err = PyErr::from_value_bound(&PyString::new_bound(py, "foo").as_borrowed());
/// assert_eq!(
/// err.to_string(),
/// "TypeError: exceptions must derive from BaseException"
/// );
/// });
/// ```
pub fn from_value_bound(obj: Bound<'_, PyAny>) -> PyErr {
pub fn from_value_bound(obj: &Bound<'_, PyAny>) -> PyErr {
let state = if let Ok(obj) = obj.downcast::<PyBaseException>() {
PyErrState::normalized(obj.clone())
} else {
Expand Down Expand Up @@ -738,10 +737,12 @@ impl PyErr {
/// Return the cause (either an exception instance, or None, set by `raise ... from ...`)
/// associated with the exception, as accessible from Python through `__cause__`.
pub fn cause(&self, py: Python<'_>) -> Option<PyErr> {
let value = self.value_bound(py);
let obj =
unsafe { py.from_owned_ptr_or_opt::<PyAny>(ffi::PyException_GetCause(value.as_ptr())) };
obj.map(Self::from_value)
let obj = unsafe {
py.from_owned_ptr_or_opt::<PyAny>(ffi::PyException_GetCause(
self.value_bound(py).as_ptr(),
))
};
obj.map(|inner| Self::from_value_bound(&inner.as_borrowed()))
}

/// Set the cause associated with the exception, pass `None` to clear it.
Expand Down
9 changes: 7 additions & 2 deletions src/exceptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ macro_rules! impl_exception_boilerplate {
impl ::std::convert::From<&$name> for $crate::PyErr {
#[inline]
fn from(err: &$name) -> $crate::PyErr {
$crate::PyErr::from_value(err)
use $crate::PyNativeType;
$crate::PyErr::from_value_bound(&err.as_borrowed())
}
}

Expand Down Expand Up @@ -1079,7 +1080,11 @@ mod tests {
let invalid_utf8 = b"fo\xd8o";
#[cfg_attr(invalid_from_utf8_lint, allow(invalid_from_utf8))]
let err = std::str::from_utf8(invalid_utf8).expect_err("should be invalid utf8");
PyErr::from_value(PyUnicodeDecodeError::new_utf8(py, invalid_utf8, err).unwrap())
PyErr::from_value_bound(
&PyUnicodeDecodeError::new_utf8(py, invalid_utf8, err)
.unwrap()
.as_borrowed(),
)
});
test_exception!(PyUnicodeEncodeError, |py| py
.eval_bound("chr(40960).encode('ascii')", None, None)
Expand Down
3 changes: 2 additions & 1 deletion src/tests/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ mod inner {
#[pymethods(crate = "pyo3")]
impl UnraisableCapture {
pub fn hook(&mut self, unraisable: &PyAny) {
let err = PyErr::from_value(unraisable.getattr("exc_value").unwrap());
let err =
PyErr::from_value_bound(&unraisable.getattr("exc_value").unwrap().as_borrowed());
let instance = unraisable.getattr("object").unwrap();
self.capture = Some((err, instance.into()));
}
Expand Down
40 changes: 23 additions & 17 deletions src/types/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,34 +73,40 @@ impl<'a> PyStringData<'a> {
match self {
Self::Ucs1(data) => match str::from_utf8(data) {
Ok(s) => Ok(Cow::Borrowed(s)),
Err(e) => Err(crate::PyErr::from_value(PyUnicodeDecodeError::new_utf8(
py, data, e,
)?)),
Err(e) => Err(crate::PyErr::from_value_bound(
&PyUnicodeDecodeError::new_utf8(py, data, e)?.as_borrowed(),
)),
},
Self::Ucs2(data) => match String::from_utf16(data) {
Ok(s) => Ok(Cow::Owned(s)),
Err(e) => {
let mut message = e.to_string().as_bytes().to_vec();
message.push(0);

Err(crate::PyErr::from_value(PyUnicodeDecodeError::new(
py,
CStr::from_bytes_with_nul(b"utf-16\0").unwrap(),
self.as_bytes(),
0..self.as_bytes().len(),
CStr::from_bytes_with_nul(&message).unwrap(),
)?))
Err(crate::PyErr::from_value_bound(
&PyUnicodeDecodeError::new(
py,
CStr::from_bytes_with_nul(b"utf-16\0").unwrap(),
self.as_bytes(),
0..self.as_bytes().len(),
CStr::from_bytes_with_nul(&message).unwrap(),
)?
.as_borrowed(),
))
}
},
Self::Ucs4(data) => match data.iter().map(|&c| std::char::from_u32(c)).collect() {
Some(s) => Ok(Cow::Owned(s)),
None => Err(crate::PyErr::from_value(PyUnicodeDecodeError::new(
py,
CStr::from_bytes_with_nul(b"utf-32\0").unwrap(),
self.as_bytes(),
0..self.as_bytes().len(),
CStr::from_bytes_with_nul(b"error converting utf-32\0").unwrap(),
)?)),
None => Err(crate::PyErr::from_value_bound(
&PyUnicodeDecodeError::new(
py,
CStr::from_bytes_with_nul(b"utf-32\0").unwrap(),
self.as_bytes(),
0..self.as_bytes().len(),
CStr::from_bytes_with_nul(b"error converting utf-32\0").unwrap(),
)?
.as_borrowed(),
)),
},
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/types/traceback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ except Exception as e:
Some(&locals),
)
.unwrap();
let err = PyErr::from_value(locals.get_item("err").unwrap().unwrap().into_gil_ref());
let err = PyErr::from_value_bound(&locals.get_item("err").unwrap().unwrap());
let traceback = err.value_bound(py).getattr("__traceback__").unwrap();
assert!(err.traceback_bound(py).unwrap().is(&traceback));
})
Expand Down

0 comments on commit 6371ce1

Please sign in to comment.