diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 9d7d080d844..9494af20ba7 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -432,13 +432,15 @@ fn impl_traverse_slot( ctx: &Ctx, ) -> syn::Result { let Ctx { pyo3_path } = ctx; - if let (Some(py_arg), _) = split_off_python_arg(&spec.signature.arguments) { + let (py_arg, args) = split_off_python_arg(&spec.signature.arguments); + if let Some(py_arg) = py_arg { return Err(syn::Error::new_spanned(py_arg.ty, "__traverse__ may not take `Python`. \ - Usually, an implementation of `__traverse__` should do nothing but calls to `visit.call`. \ - Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, \ - i.e. `Python::with_gil` will panic.")); + Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` \ + should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited \ + inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic.")); } + // check that the receiver does not try to smuggle an (implicit) `Python` token into here if let FnType::Fn(SelfType::TryFromBoundRef(span)) | FnType::Fn(SelfType::Receiver { mutable: true, @@ -446,12 +448,22 @@ fn impl_traverse_slot( }) = spec.tp { bail_spanned! { span => - "__traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__` \ + "__traverse__ may not take a receiver other than `&self`. Usually, an implementation of \ + `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` \ should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited \ inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic." } } + // check that we have a single argument (other than `&self`) of type `PyVisit` + if !matches!(args, [FnArg { ty: syn::Type::Path(p), .. }] if p.path.segments.last().map(|seg| seg.ident == "PyVisit").unwrap_or(false)) + { + bail_spanned! { spec.name.span() => + "`__traverse__` has to have the following signature: \ + `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>`" + } + } + let rust_fn_ident = spec.name; let associated_method = quote! { diff --git a/tests/ui/traverse.rs b/tests/ui/traverse.rs index 0cf7170db21..5685b2ffb04 100644 --- a/tests/ui/traverse.rs +++ b/tests/ui/traverse.rs @@ -52,6 +52,36 @@ impl TraverseTriesToTakeSelf { } } +#[pyclass] +struct TraverseMissingPyVisit {} + +#[pymethods] +impl TraverseMissingPyVisit { + fn __traverse__(&self) -> Result<(), PyTraverseError> { + Ok(()) + } +} + +#[pyclass] +struct TraverseWrongArg {} + +#[pymethods] +impl TraverseWrongArg { + fn __traverse__(&self, arg: &Bound<'_, PyAny>) -> Result<(), PyTraverseError> { + Ok(()) + } +} + +#[pyclass] +struct TraverseAdditionalArg {} + +#[pymethods] +impl TraverseAdditionalArg { + fn __traverse__(&self, visit: PyVisit, arg: &Bound<'_, PyAny>) -> Result<(), PyTraverseError> { + Ok(()) + } +} + #[pyclass] struct Class; diff --git a/tests/ui/traverse.stderr b/tests/ui/traverse.stderr index a5f8c45a93e..dae9fa94845 100644 --- a/tests/ui/traverse.stderr +++ b/tests/ui/traverse.stderr @@ -1,29 +1,47 @@ -error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. +error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. --> tests/ui/traverse.rs:10:26 | 10 | fn __traverse__(slf: PyRef, visit: PyVisit) -> Result<(), PyTraverseError> { | ^^^^^ -error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. +error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. --> tests/ui/traverse.rs:20:26 | 20 | fn __traverse__(slf: PyRefMut, visit: PyVisit) -> Result<(), PyTraverseError> { | ^^^^^^^^ -error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. +error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. --> tests/ui/traverse.rs:30:26 | 30 | fn __traverse__(slf: Bound<'_, Self>, visit: PyVisit) -> Result<(), PyTraverseError> { | ^^^^^ -error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. +error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. --> tests/ui/traverse.rs:40:21 | 40 | fn __traverse__(&mut self, visit: PyVisit) -> Result<(), PyTraverseError> { | ^ -error: __traverse__ may not take `Python`. Usually, an implementation of `__traverse__` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. - --> tests/ui/traverse.rs:60:32 +error: `__traverse__` has to have the following signature: `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` + --> tests/ui/traverse.rs:60:8 | -60 | fn __traverse__(&self, py: Python<'_>, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { +60 | fn __traverse__(&self) -> Result<(), PyTraverseError> { + | ^^^^^^^^^^^^ + +error: `__traverse__` has to have the following signature: `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` + --> tests/ui/traverse.rs:70:8 + | +70 | fn __traverse__(&self, arg: &Bound<'_, PyAny>) -> Result<(), PyTraverseError> { + | ^^^^^^^^^^^^ + +error: `__traverse__` has to have the following signature: `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` + --> tests/ui/traverse.rs:80:8 + | +80 | fn __traverse__(&self, visit: PyVisit, arg: &Bound<'_, PyAny>) -> Result<(), PyTraverseError> { + | ^^^^^^^^^^^^ + +error: __traverse__ may not take `Python`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic. + --> tests/ui/traverse.rs:90:32 + | +90 | fn __traverse__(&self, py: Python<'_>, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { | ^^^^^^^^^^