Skip to content

Commit

Permalink
Avoid attribute lookup overhead for __bool__ if the unlimited API is …
Browse files Browse the repository at this point in the history
…available.
  • Loading branch information
adamreichold committed Dec 10, 2023
1 parent 9e348a9 commit 556271e
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 10 deletions.
1 change: 1 addition & 0 deletions src/types/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ impl PyAny {
///
/// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used
/// to intern `attr_name`.
#[allow(dead_code)] // Currently only used with num-complex+abi3, so dead without that.
pub(crate) fn lookup_special<N>(&self, attr_name: N) -> PyResult<Option<&PyAny>>
where
N: IntoPy<Py<PyString>>,
Expand Down
41 changes: 31 additions & 10 deletions src/types/boolobject.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#[cfg(feature = "experimental-inspect")]
use crate::inspect::types::TypeInfo;
use crate::{
exceptions::PyTypeError, ffi, intern, FromPyObject, IntoPy, PyAny, PyObject, PyResult, Python,
exceptions::PyTypeError, ffi, FromPyObject, IntoPy, PyAny, PyObject, PyResult, Python,
ToPyObject,
};

Expand Down Expand Up @@ -63,12 +63,32 @@ impl<'source> FromPyObject<'source> for bool {
return Ok(obj.is_true());
}

let meth = obj
.lookup_special(intern!(obj.py(), "__bool__"))?
.ok_or_else(|| PyTypeError::new_err("object has no __bool__ magic method"))?;
#[cfg(not(any(Py_LIMITED_API, PyPy)))]
unsafe {
let ptr = obj.as_ptr();

if let Some(tp_as_number) = (*(*ptr).ob_type).tp_as_number.as_ref() {
if let Some(nb_bool) = tp_as_number.nb_bool {
match (nb_bool)(ptr) {
0 => return Ok(false),
1 => return Ok(true),
_ => return Err(crate::PyErr::fetch(obj.py())),
}
}
}

Err(PyTypeError::new_err("object has no __bool__ magic method"))
}

#[cfg(any(Py_LIMITED_API, PyPy))]
{
let meth = obj
.lookup_special(crate::intern!(obj.py(), "__bool__"))?
.ok_or_else(|| PyTypeError::new_err("object has no __bool__ magic method"))?;

let obj = meth.call0()?.downcast::<PyBool>()?;
Ok(obj.is_true())
let obj = meth.call0()?.downcast::<PyBool>()?;
Ok(obj.is_true())
}
}

#[cfg(feature = "experimental-inspect")]
Expand Down Expand Up @@ -127,10 +147,11 @@ class D:
assert!(a.extract::<bool>().unwrap());

let b = module.getattr("B").unwrap().call0().unwrap();
assert_eq!(
b.extract::<bool>().unwrap_err().to_string(),
"TypeError: 'str' object cannot be converted to 'PyBool'",
);
assert!(matches!(
&*b.extract::<bool>().unwrap_err().to_string(),
"TypeError: 'str' object cannot be converted to 'PyBool'"
| "TypeError: __bool__ should return bool, returned str"
));

let c = module.getattr("C").unwrap().call0().unwrap();
assert_eq!(
Expand Down

0 comments on commit 556271e

Please # to comment.