fix unintentional unsafe_op_in_unsafe_fn trigger (#4674)

* fix unintentional `unsafe_op_in_unsafe_fn` trigger * add newsfragment
PyO3 · Nov 2, 2024 · fdb29cc · fdb29cc
1 parent c0e6232
commit fdb29cc
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 35 deletions.
diff --git a/newsfragments/4674.fixed.md b/newsfragments/4674.fixed.md
@@ -0,0 +1 @@
+Fixes unintentional `unsafe_op_in_unsafe_fn` trigger by adjusting macro hygiene.
diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs
@@ -270,9 +270,9 @@ impl FnType {
                     ::std::convert::Into::into(
                         #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(#py, &*(&#slf as *const _ as *const *mut _))
                             .downcast_unchecked::<#pyo3_path::types::PyType>()
-                    ),
+                    )
                 };
-                Some(ret)
+                Some(quote! { unsafe { #ret }, })
             }
             FnType::FnModule(span) => {
                 let py = syn::Ident::new("py", Span::call_site());
@@ -283,9 +283,9 @@ impl FnType {
                     ::std::convert::Into::into(
                         #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(#py, &*(&#slf as *const _ as *const *mut _))
                             .downcast_unchecked::<#pyo3_path::types::PyModule>()
-                    ),
+                    )
                 };
-                Some(ret)
+                Some(quote! { unsafe { #ret }, })
             }
             FnType::FnNew | FnType::FnStatic | FnType::ClassAttribute => None,
         }
@@ -332,6 +332,8 @@ impl SelfType {
         let py = syn::Ident::new("py", Span::call_site());
         let slf = syn::Ident::new("_slf", Span::call_site());
         let Ctx { pyo3_path, .. } = ctx;
+        let bound_ref =
+            quote! { unsafe { #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(#py, &#slf) } };
         match self {
             SelfType::Receiver { span, mutable } => {
                 let method = if *mutable {
@@ -344,7 +346,7 @@ impl SelfType {
                 error_mode.handle_error(
                     quote_spanned! { *span =>
                         #pyo3_path::impl_::extract_argument::#method::<#cls>(
-                            #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(#py, &#slf).0,
+                            #bound_ref.0,
                             &mut #holder,
                         )
                     },
@@ -355,7 +357,7 @@ impl SelfType {
                 let pyo3_path = pyo3_path.to_tokens_spanned(*span);
                 error_mode.handle_error(
                     quote_spanned! { *span =>
-                        #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(#py, &#slf).downcast::<#cls>()
+                        #bound_ref.downcast::<#cls>()
                             .map_err(::std::convert::Into::<#pyo3_path::PyErr>::into)
                             .and_then(
                                 #[allow(unknown_lints, clippy::unnecessary_fallible_conversions)]  // In case slf is Py<Self> (unknown_lints can be removed when MSRV is 1.75+)
@@ -665,14 +667,14 @@ impl<'a> FnSpec<'a> {
                     FnType::Fn(SelfType::Receiver { mutable: false, .. }) => {
                         quote! {{
                             #(let #arg_names = #args;)*
-                            let __guard = #pyo3_path::impl_::coroutine::RefGuard::<#cls>::new(&#pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &_slf))?;
+                            let __guard = unsafe { #pyo3_path::impl_::coroutine::RefGuard::<#cls>::new(&#pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &_slf))? };
                             async move { function(&__guard, #(#arg_names),*).await }
                         }}
                     }
                     FnType::Fn(SelfType::Receiver { mutable: true, .. }) => {
                         quote! {{
                             #(let #arg_names = #args;)*
-                            let mut __guard = #pyo3_path::impl_::coroutine::RefMutGuard::<#cls>::new(&#pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &_slf))?;
+                            let mut __guard = unsafe { #pyo3_path::impl_::coroutine::RefMutGuard::<#cls>::new(&#pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &_slf))? };
                             async move { function(&mut __guard, #(#arg_names),*).await }
                         }}
                     }
@@ -862,11 +864,13 @@ impl<'a> FnSpec<'a> {
                             _args: *mut #pyo3_path::ffi::PyObject,
                         ) -> *mut #pyo3_path::ffi::PyObject
                         {
-                            #pyo3_path::impl_::trampoline::noargs(
-                                _slf,
-                                _args,
-                                #wrapper
-                            )
+                            unsafe {
+                                #pyo3_path::impl_::trampoline::noargs(
+                                    _slf,
+                                    _args,
+                                    #wrapper
+                                )
+                            }
                         }
                         trampoline
                     },

diff --git a/pyo3-macros-backend/src/module.rs b/pyo3-macros-backend/src/module.rs
@@ -449,7 +449,7 @@ fn module_initialization(
             #[doc(hidden)]
             #[export_name = #pyinit_symbol]
             pub unsafe extern "C" fn __pyo3_init() -> *mut #pyo3_path::ffi::PyObject {
-                #pyo3_path::impl_::trampoline::module_init(|py| _PYO3_DEF.make_module(py))
+                unsafe { #pyo3_path::impl_::trampoline::module_init(|py| _PYO3_DEF.make_module(py)) }
             }
         });
     }

diff --git a/pyo3-macros-backend/src/params.rs b/pyo3-macros-backend/src/params.rs
@@ -79,7 +79,7 @@ pub fn impl_arg_params(
             .collect();
         return (
             quote! {
-                let _args = #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &_args);
+                let _args = unsafe { #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &_args) };
                 let _kwargs = #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr_or_opt(py, &_kwargs);
                 #from_py_with
             },
@@ -301,9 +301,10 @@ pub(crate) fn impl_regular_arg_param(
         }
     } else {
         let holder = holders.push_holder(arg.name.span());
+        let unwrap = quote! {unsafe { #pyo3_path::impl_::extract_argument::unwrap_required_argument(#arg_value) }};
         quote_arg_span! {
             #pyo3_path::impl_::extract_argument::extract_argument(
-                #pyo3_path::impl_::extract_argument::unwrap_required_argument(#arg_value),
+                #unwrap,
                 &mut #holder,
                 #name_str
             )?

diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs
@@ -1182,25 +1182,26 @@ fn extract_object(
     let Ctx { pyo3_path, .. } = ctx;
     let name = arg.name().unraw().to_string();
 
-    let extract =
-        if let Some(from_py_with) = arg.from_py_with().map(|from_py_with| &from_py_with.value) {
-            quote! {
-                #pyo3_path::impl_::extract_argument::from_py_with(
-                    #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &#source_ptr).0,
-                    #name,
-                    #from_py_with as fn(_) -> _,
-                )
-            }
-        } else {
-            let holder = holders.push_holder(Span::call_site());
-            quote! {
-                #pyo3_path::impl_::extract_argument::extract_argument(
-                    #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &#source_ptr).0,
-                    &mut #holder,
-                    #name
-                )
-            }
-        };
+    let extract = if let Some(from_py_with) =
+        arg.from_py_with().map(|from_py_with| &from_py_with.value)
+    {
+        quote! {
+            #pyo3_path::impl_::extract_argument::from_py_with(
+                unsafe { #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &#source_ptr).0 },
+                #name,
+                #from_py_with as fn(_) -> _,
+            )
+        }
+    } else {
+        let holder = holders.push_holder(Span::call_site());
+        quote! {
+            #pyo3_path::impl_::extract_argument::extract_argument(
+                unsafe { #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &#source_ptr).0 },
+                &mut #holder,
+                #name
+            )
+        }
+    };
 
     let extracted = extract_error_mode.handle_error(extract, ctx);
     quote!(#extracted)

diff --git a/tests/ui/forbid_unsafe.rs b/tests/ui/forbid_unsafe.rs
@@ -1,4 +1,5 @@
 #![forbid(unsafe_code)]
+#![forbid(unsafe_op_in_unsafe_fn)]
 
 use pyo3::*;