fix unintentional unsafe_code trigger (#4574)

* Revert "Add missing #[allow(unsafe_code)] attributes (#4396)"

This reverts commit 0e03b39.

* fix unintentional `unsafe_code` trigger

* add newsfragment

newsfragments/4574.fixed.md

@@ -0,0 +1,2 @@
+Fixes `#[forbid(unsafe_code)]` regression by reverting #4396.
+Fixes unintentional `unsafe_code` trigger by adjusting macro hygiene.

pyo3-macros-backend/src/pyclass.rs

@@ -1659,7 +1659,7 @@ fn impl_pytypeinfo(
 
     #[cfg(feature = "gil-refs")]
     let has_py_gil_ref = quote! {
-        #[allow(deprecated, unsafe_code)]
+        #[allow(deprecated)]
         unsafe impl #pyo3_path::type_object::HasPyGilRef for #cls {
             type AsRefTarget = #pyo3_path::PyCell<Self>;
         }
@@ -1671,7 +1671,6 @@ fn impl_pytypeinfo(
     quote! {
         #has_py_gil_ref
 
-        #[allow(unsafe_code)]
         unsafe impl #pyo3_path::type_object::PyTypeInfo for #cls {
             const NAME: &'static str = #cls_name;
             const MODULE: ::std::option::Option<&'static str> = #module;

pyo3-macros-backend/src/pymethod.rs

@@ -781,32 +781,35 @@ pub fn impl_py_getter_def(
 
             // TODO: on MSRV 1.77+, we can use `::std::mem::offset_of!` here, and it should
             // make it possible for the `MaybeRuntimePyMethodDef` to be a `Static` variant.
-            let method_def = quote_spanned! {ty.span()=>
+            let generator = quote_spanned! { ty.span() =>
+                #pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Runtime(
+                    || GENERATOR.generate(#python_name, #doc)
+                )
+            };
+            // This is separate so that the unsafe below does not inherit the span and thus does not
+            // trigger the `unsafe_code` lint
+            let method_def = quote! {
                 #cfg_attrs
                 {
                     #[allow(unused_imports)]  // might not be used if all probes are positve
                     use #pyo3_path::impl_::pyclass::Probe;
 
                     struct Offset;
-                    #[allow(unsafe_code)]
                     unsafe impl #pyo3_path::impl_::pyclass::OffsetCalculator<#cls, #ty> for Offset {
                         fn offset() -> usize {
                             #pyo3_path::impl_::pyclass::class_offset::<#cls>() +
                             #pyo3_path::impl_::pyclass::offset_of!(#cls, #field)
                         }
                     }
 
-                    #[allow(unsafe_code)]
                     const GENERATOR: #pyo3_path::impl_::pyclass::PyClassGetterGenerator::<
                         #cls,
                         #ty,
                         Offset,
                         { #pyo3_path::impl_::pyclass::IsPyT::<#ty>::VALUE },
                         { #pyo3_path::impl_::pyclass::IsToPyObject::<#ty>::VALUE },
                     > = unsafe { #pyo3_path::impl_::pyclass::PyClassGetterGenerator::new() };
-                    #pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Runtime(
-                        || GENERATOR.generate(#python_name, #doc)
-                    )
+                    #generator
                 }
             };
 

src/exceptions.rs

@@ -34,7 +34,6 @@ macro_rules! impl_exception_boilerplate {
         #[cfg(feature = "gil-refs")]
         impl ::std::error::Error for $name {
             fn source(&self) -> ::std::option::Option<&(dyn ::std::error::Error + 'static)> {
-                #[allow(unsafe_code)]
                 unsafe {
                     #[allow(deprecated)]
                     let cause: &$crate::exceptions::PyBaseException = self

src/impl_/pyclass.rs

@@ -353,7 +353,6 @@ slot_fragment_trait! {
 #[macro_export]
 macro_rules! generate_pyclass_getattro_slot {
     ($cls:ty) => {{
-        #[allow(unsafe_code)]
         unsafe extern "C" fn __wrap(
             _slf: *mut $crate::ffi::PyObject,
             attr: *mut $crate::ffi::PyObject,
@@ -437,7 +436,6 @@ macro_rules! define_pyclass_setattr_slot {
         #[macro_export]
         macro_rules! $generate_macro {
             ($cls:ty) => {{
-                #[allow(unsafe_code)]
                 unsafe extern "C" fn __wrap(
                     _slf: *mut $crate::ffi::PyObject,
                     attr: *mut $crate::ffi::PyObject,
@@ -554,7 +552,6 @@ macro_rules! define_pyclass_binary_operator_slot {
         #[macro_export]
         macro_rules! $generate_macro {
             ($cls:ty) => {{
-                #[allow(unsafe_code)]
                 unsafe extern "C" fn __wrap(
                     _slf: *mut $crate::ffi::PyObject,
                     _other: *mut $crate::ffi::PyObject,
@@ -747,7 +744,6 @@ slot_fragment_trait! {
 #[macro_export]
 macro_rules! generate_pyclass_pow_slot {
     ($cls:ty) => {{
-        #[allow(unsafe_code)]
         unsafe extern "C" fn __wrap(
             _slf: *mut $crate::ffi::PyObject,
             _other: *mut $crate::ffi::PyObject,
@@ -872,7 +868,7 @@ macro_rules! generate_pyclass_richcompare_slot {
     ($cls:ty) => {{
         #[allow(unknown_lints, non_local_definitions)]
         impl $cls {
-            #[allow(non_snake_case, unsafe_code)]
+            #[allow(non_snake_case)]
             unsafe extern "C" fn __pymethod___richcmp____(
                 slf: *mut $crate::ffi::PyObject,
                 other: *mut $crate::ffi::PyObject,

src/macros.rs

@@ -207,7 +207,6 @@ macro_rules! wrap_pymodule {
 #[macro_export]
 macro_rules! append_to_inittab {
     ($module:ident) => {
-        #[allow(unsafe_code)]
         unsafe {
             if $crate::ffi::Py_IsInitialized() != 0 {
                 ::std::panic!(

src/tests/hygiene/mod.rs

@@ -1,6 +1,5 @@
 #![no_implicit_prelude]
 #![allow(dead_code, unused_variables, clippy::unnecessary_wraps)]
-#![deny(unsafe_code)]
 
 // The modules in this test are used to check PyO3 macro expansion is hygienic. By locating the test
 // inside the crate the global `::pyo3` namespace is not available, so in combination with

src/types/mod.rs

@@ -124,7 +124,6 @@ pub trait DerefToPyAny {
 macro_rules! pyobject_native_type_base(
     ($name:ty $(;$generics:ident)* ) => {
         #[cfg(feature = "gil-refs")]
-        #[allow(unsafe_code)]
         unsafe impl<$($generics,)*> $crate::PyNativeType for $name {
             type AsRefSource = Self;
         }
@@ -163,7 +162,6 @@ macro_rules! pyobject_native_type_base(
         {
             #[inline]
             fn to_object(&self, py: $crate::Python<'_>) -> $crate::PyObject {
-                #[allow(unsafe_code)]
                 unsafe { $crate::PyObject::from_borrowed_ptr(py, self.as_ptr()) }
             }
         }
@@ -194,7 +192,6 @@ macro_rules! pyobject_native_type_named (
             }
         }
 
-        #[allow(unsafe_code)]
         unsafe impl<$($generics,)*> $crate::AsPyPointer for $name {
             /// Gets the underlying FFI pointer, returns a borrowed pointer.
             #[inline]
@@ -209,7 +206,6 @@ macro_rules! pyobject_native_type_named (
         impl<$($generics,)*> $crate::IntoPy<$crate::Py<$name>> for &'_ $name {
             #[inline]
             fn into_py(self, py: $crate::Python<'_>) -> $crate::Py<$name> {
-                #[allow(unsafe_code)]
                 unsafe { $crate::Py::from_borrowed_ptr(py, self.as_ptr()) }
             }
         }
@@ -221,7 +217,6 @@ macro_rules! pyobject_native_type_named (
             #[inline]
             fn from(other: &$name) -> Self {
                 use $crate::PyNativeType;
-                #[allow(unsafe_code)]
                 unsafe { $crate::Py::from_borrowed_ptr(other.py(), other.as_ptr()) }
             }
         }
@@ -231,7 +226,6 @@ macro_rules! pyobject_native_type_named (
         #[cfg(feature = "gil-refs")]
         impl<'a, $($generics,)*> ::std::convert::From<&'a $name> for &'a $crate::PyAny {
             fn from(ob: &'a $name) -> Self {
-                #[allow(unsafe_code)]
                 unsafe{&*(ob as *const $name as *const $crate::PyAny)}
             }
         }
@@ -255,7 +249,6 @@ macro_rules! pyobject_native_static_type_object(
 #[macro_export]
 macro_rules! pyobject_native_type_info(
     ($name:ty, $typeobject:expr, $module:expr $(, #checkfunction=$checkfunction:path)? $(;$generics:ident)*) => {
-        #[allow(unsafe_code)]
         unsafe impl<$($generics,)*> $crate::type_object::PyTypeInfo for $name {
             const NAME: &'static str = stringify!($name);
             const MODULE: ::std::option::Option<&'static str> = $module;

tests/test_compile_error.rs

@@ -63,6 +63,8 @@ fn test_compile_errors() {
     #[cfg(any(not(Py_LIMITED_API), Py_3_10))] // to avoid PyFunctionArgument for &str
     t.compile_fail("tests/ui/invalid_cancel_handle.rs");
     t.pass("tests/ui/pymodule_missing_docs.rs");
+    #[cfg(not(Py_LIMITED_API))]
+    t.pass("tests/ui/forbid_unsafe.rs");
     #[cfg(all(Py_LIMITED_API, not(feature = "experimental-async")))]
     // output changes with async feature
     t.compile_fail("tests/ui/abi3_inheritance.rs");

tests/ui/forbid_unsafe.rs

@@ -0,0 +1,30 @@
+#![forbid(unsafe_code)]
+
+use pyo3::*;
+
+#[allow(unexpected_cfgs)]
+#[path = "../../src/tests/hygiene/mod.rs"]
+mod hygiene;
+
+mod gh_4394 {
+    use pyo3::prelude::*;
+
+    #[derive(Eq, Ord, PartialEq, PartialOrd, Clone)]
+    #[pyclass(get_all)]
+    pub struct VersionSpecifier {
+        pub(crate) operator: Operator,
+        pub(crate) version: Version,
+    }
+
+    #[derive(Eq, Ord, PartialEq, PartialOrd, Debug, Hash, Clone, Copy)]
+    #[pyo3::pyclass(eq, eq_int)]
+    pub enum Operator {
+        Equal,
+    }
+
+    #[derive(Clone, Eq, PartialEq, PartialOrd, Ord)]
+    #[pyclass]
+    pub struct Version;
+}
+
+fn main() {}