Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

verify py method args count #2083

Merged
merged 4 commits into from
Jan 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
where to find the `pyo3` crate, in case it has been renamed or is re-exported and not found at the crate root. [#2022](https://github.com/PyO3/pyo3/pull/2022)
- Expose `pyo3-build-config` APIs for cross-compiling and Python configuration discovery for use in other projects. [#1996](https://github.com/PyO3/pyo3/pull/1996)
- Accept paths in `wrap_pyfunction` and `wrap_pymodule`. [#2081](https://github.com/PyO3/pyo3/pull/2081)
- Add check for correct number of arguments on magic methods. [#2083](https://github.com/PyO3/pyo3/pull/2083)

### Changed

Expand All @@ -45,6 +46,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- internal `handle_panic` helper [#2074](https://github.com/PyO3/pyo3/pull/2074)
- `#[pyfunction]` and `#[pymethods]` argument extraction [#2075](https://github.com/PyO3/pyo3/pull/2075)
- `#[pyclass]` type object creation [#2076](https://github.com/PyO3/pyo3/pull/2076) [#2081](https://github.com/PyO3/pyo3/pull/2081)
- `__ipow__` now supports modulo argument on Python 3.8+. [#2083](https://github.com/PyO3/pyo3/pull/2083)
- `pyo3-macros-backend` is now compiled with PyO3 cfgs to enable different magic method definitions based on version. [#2083](https://github.com/PyO3/pyo3/pull/2083)

### Removed

Expand Down
4 changes: 3 additions & 1 deletion guide/src/class/protocols.md
Original file line number Diff line number Diff line change
Expand Up @@ -288,13 +288,15 @@ This trait also has support the augmented arithmetic assignments (`+=`, `-=`,
* `fn __itruediv__(&'p mut self, other: impl FromPyObject) -> PyResult<()>`
* `fn __ifloordiv__(&'p mut self, other: impl FromPyObject) -> PyResult<()>`
* `fn __imod__(&'p mut self, other: impl FromPyObject) -> PyResult<()>`
* `fn __ipow__(&'p mut self, other: impl FromPyObject) -> PyResult<()>`
* `fn __ipow__(&'p mut self, other: impl FromPyObject, modulo: impl FromPyObject) -> PyResult<()>` on Python 3.8^
* `fn __ipow__(&'p mut self, other: impl FromPyObject) -> PyResult<()>` on Python 3.7 see https://bugs.python.org/issue36379
* `fn __ilshift__(&'p mut self, other: impl FromPyObject) -> PyResult<()>`
* `fn __irshift__(&'p mut self, other: impl FromPyObject) -> PyResult<()>`
* `fn __iand__(&'p mut self, other: impl FromPyObject) -> PyResult<()>`
* `fn __ior__(&'p mut self, other: impl FromPyObject) -> PyResult<()>`
* `fn __ixor__(&'p mut self, other: impl FromPyObject) -> PyResult<()>`


The following methods implement the unary arithmetic operations (`-`, `+`, `abs()` and `~`):

* `fn __neg__(&'p self) -> PyResult<impl ToPyObject>`
Expand Down
2 changes: 1 addition & 1 deletion pyo3-macros-backend/src/defs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ pub const NUM: Proto = Proto {
.args(&["Other"])
.has_self(),
MethodProto::new("__ipow__", "PyNumberIPowProtocol")
.args(&["Other"])
.args(&["Other", "Modulo"])
.has_self(),
MethodProto::new("__ilshift__", "PyNumberILShiftProtocol")
.args(&["Other"])
Expand Down
26 changes: 20 additions & 6 deletions pyo3-macros-backend/src/pymethod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,8 +532,8 @@ const __IMOD__: SlotDef = SlotDef::new("Py_nb_inplace_remainder", "binaryfunc")
.arguments(&[Ty::Object])
.extract_error_mode(ExtractErrorMode::NotImplemented)
.return_self();
const __IPOW__: SlotDef = SlotDef::new("Py_nb_inplace_power", "ternaryfunc")
.arguments(&[Ty::Object, Ty::Object])
const __IPOW__: SlotDef = SlotDef::new("Py_nb_inplace_power", "ipowfunc")
.arguments(&[Ty::Object, Ty::IPowModulo])
.extract_error_mode(ExtractErrorMode::NotImplemented)
.return_self();
const __ILSHIFT__: SlotDef = SlotDef::new("Py_nb_inplace_lshift", "binaryfunc")
Expand Down Expand Up @@ -603,6 +603,7 @@ enum Ty {
Object,
MaybeNullObject,
NonNullObject,
IPowModulo,
CompareOp,
Int,
PyHashT,
Expand All @@ -615,6 +616,7 @@ impl Ty {
match self {
Ty::Object | Ty::MaybeNullObject => quote! { *mut _pyo3::ffi::PyObject },
Ty::NonNullObject => quote! { ::std::ptr::NonNull<_pyo3::ffi::PyObject> },
Ty::IPowModulo => quote! { _pyo3::impl_::pymethods::IPowModulo },
Ty::Int | Ty::CompareOp => quote! { ::std::os::raw::c_int },
Ty::PyHashT => quote! { _pyo3::ffi::Py_hash_t },
Ty::PySsizeT => quote! { _pyo3::ffi::Py_ssize_t },
Expand Down Expand Up @@ -667,6 +669,16 @@ impl Ty {
);
extract_object(cls, arg.ty, ident, extract)
}
Ty::IPowModulo => {
let extract = handle_error(
extract_error_mode,
py,
quote! {
#ident.extract(#py)
},
);
extract_object(cls, arg.ty, ident, extract)
}
Ty::CompareOp => {
let extract = handle_error(
extract_error_mode,
Expand Down Expand Up @@ -856,8 +868,11 @@ fn generate_method_body(
) -> Result<TokenStream> {
let self_conversion = spec.tp.self_conversion(Some(cls), extract_error_mode);
let rust_name = spec.name;
let (arg_idents, conversions) =
let (arg_idents, arg_count, conversions) =
extract_proto_arguments(cls, py, &spec.args, arguments, extract_error_mode)?;
if arg_count != arguments.len() {
bail_spanned!(spec.name.span() => format!("Expected {} arguments, got {}", arguments.len(), arg_count));
}
let call = quote! { _pyo3::callback::convert(#py, #cls::#rust_name(_slf, #(#arg_idents),*)) };
let body = if let Some(return_mode) = return_mode {
return_mode.return_call_output(py, call)
Expand Down Expand Up @@ -1026,7 +1041,7 @@ fn extract_proto_arguments(
method_args: &[FnArg],
proto_args: &[Ty],
extract_error_mode: ExtractErrorMode,
) -> Result<(Vec<Ident>, TokenStream)> {
) -> Result<(Vec<Ident>, usize, TokenStream)> {
let mut arg_idents = Vec::with_capacity(method_args.len());
let mut non_python_args = 0;

Expand All @@ -1045,9 +1060,8 @@ fn extract_proto_arguments(
arg_idents.push(ident);
}
}

let conversions = quote!(#(#args_conversions)*);
Ok((arg_idents, conversions))
Ok((arg_idents, non_python_args, conversions))
}

struct StaticIdent(&'static str);
Expand Down
23 changes: 18 additions & 5 deletions src/class/number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ pub trait PyNumberProtocol<'p>: PyClass {
{
unimplemented!()
}
fn __ipow__(&'p mut self, other: Self::Other) -> Self::Result
fn __ipow__(&'p mut self, other: Self::Other, modulo: Option<Self::Modulo>) -> Self::Result
where
Self: PyNumberIPowProtocol<'p>,
{
Expand Down Expand Up @@ -504,6 +504,8 @@ pub trait PyNumberIDivmodProtocol<'p>: PyNumberProtocol<'p> {
pub trait PyNumberIPowProtocol<'p>: PyNumberProtocol<'p> {
type Other: FromPyObject<'p>;
type Result: IntoPyCallbackOutput<()>;
// See https://bugs.python.org/issue36379
type Modulo: FromPyObject<'p>;
}

#[allow(clippy::upper_case_acronyms)]
Expand Down Expand Up @@ -718,17 +720,28 @@ py_binary_self_func!(imod, PyNumberIModProtocol, T::__imod__);
pub unsafe extern "C" fn ipow<T>(
slf: *mut ffi::PyObject,
other: *mut ffi::PyObject,
_modulo: *mut ffi::PyObject,
modulo: crate::impl_::pymethods::IPowModulo,
) -> *mut ffi::PyObject
where
T: for<'p> PyNumberIPowProtocol<'p>,
{
// NOTE: Somehow __ipow__ causes SIGSEGV in Python < 3.8 when we extract,
// so we ignore it. It's the same as what CPython does.
crate::callback_body!(py, {
let slf_cell = py.from_borrowed_ptr::<crate::PyCell<T>>(slf);
let other = py.from_borrowed_ptr::<crate::PyAny>(other);
call_operator_mut!(py, slf_cell, __ipow__, other).convert(py)?;
slf_cell
.try_borrow_mut()?
.__ipow__(
extract_or_return_not_implemented!(other),
match modulo.extract(py) {
Ok(value) => value,
Err(_) => {
let res = crate::ffi::Py_NotImplemented();
crate::ffi::Py_INCREF(res);
return Ok(res);
}
},
)
.convert(py)?;
ffi::Py_INCREF(slf);
Ok::<_, PyErr>(slf)
})
Expand Down
4 changes: 4 additions & 0 deletions src/ffi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,3 +204,7 @@ mod cpython;

#[cfg(not(Py_LIMITED_API))]
pub use self::cpython::*;

/// Helper to enable #\[pymethods\] to see the workaround for __ipow__ on Python 3.7
#[doc(hidden)]
pub use crate::impl_::pymethods::ipowfunc;
2 changes: 2 additions & 0 deletions src/impl_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,6 @@ pub mod frompyobject;
pub(crate) mod not_send;
#[doc(hidden)]
pub mod pyclass;
#[doc(hidden)]
pub mod pymethods;
pub mod pymodule;
33 changes: 33 additions & 0 deletions src/impl_/pymethods.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use crate::{ffi, FromPyObject, PyAny, PyResult, Python};

/// Python 3.8 and up - __ipow__ has modulo argument correctly populated.
#[cfg(Py_3_8)]
#[repr(transparent)]
pub struct IPowModulo(*mut ffi::PyObject);

/// Python 3.7 and older - __ipow__ does not have modulo argument correctly populated.
#[cfg(not(Py_3_8))]
#[repr(transparent)]
pub struct IPowModulo(std::mem::MaybeUninit<*mut ffi::PyObject>);

/// Helper to use as pymethod ffi definition
#[allow(non_camel_case_types)]
pub type ipowfunc = unsafe extern "C" fn(
arg1: *mut ffi::PyObject,
arg2: *mut ffi::PyObject,
arg3: IPowModulo,
) -> *mut ffi::PyObject;

impl IPowModulo {
#[cfg(Py_3_8)]
#[inline]
pub fn extract<'a, T: FromPyObject<'a>>(self, py: Python<'a>) -> PyResult<T> {
unsafe { py.from_borrowed_ptr::<PyAny>(self.0) }.extract()
}

#[cfg(not(Py_3_8))]
#[inline]
pub fn extract<'a, T: FromPyObject<'a>>(self, py: Python<'a>) -> PyResult<T> {
unsafe { py.from_borrowed_ptr::<PyAny>(ffi::Py_None()) }.extract()
}
}
Loading