From 3f6ab7c1a502ecc7cba909b62165969d4150586e Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sun, 22 Sep 2024 12:29:26 +0200 Subject: [PATCH 1/3] Revert "Add missing #[allow(unsafe_code)] attributes (#4396)" This reverts commit 0e03b39caa18d016a6951307b297ca7e6f999e24. --- newsfragments/4396.fixed.md | 1 - pyo3-macros-backend/src/pyclass.rs | 1 - pyo3-macros-backend/src/pymethod.rs | 2 -- src/impl_/pyclass.rs | 6 +----- src/macros.rs | 1 - src/tests/hygiene/mod.rs | 1 - src/types/mod.rs | 1 - 7 files changed, 1 insertion(+), 12 deletions(-) delete mode 100644 newsfragments/4396.fixed.md diff --git a/newsfragments/4396.fixed.md b/newsfragments/4396.fixed.md deleted file mode 100644 index 285358ad526..00000000000 --- a/newsfragments/4396.fixed.md +++ /dev/null @@ -1 +0,0 @@ -Hide confusing warnings about unsafe usage in `#[pyclass]` implementation. diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 291aeb0125a..1da9cfa20ad 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -1809,7 +1809,6 @@ fn impl_pytypeinfo(cls: &syn::Ident, attr: &PyClassArgs, ctx: &Ctx) -> TokenStre }; quote! { - #[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; diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index aa0f3cedfcb..a11e68fe2a1 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -773,7 +773,6 @@ pub fn impl_py_getter_def( 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>() + @@ -781,7 +780,6 @@ pub fn impl_py_getter_def( } } - #[allow(unsafe_code)] const GENERATOR: #pyo3_path::impl_::pyclass::PyClassGetterGenerator::< #cls, #ty, diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index f116e608d2f..12c2b09b9ca 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -351,7 +351,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, @@ -435,7 +434,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, @@ -552,7 +550,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, @@ -745,7 +742,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, @@ -870,7 +866,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, diff --git a/src/macros.rs b/src/macros.rs index 1a021998bcf..e4ae4c9dc16 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -184,7 +184,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!( diff --git a/src/tests/hygiene/mod.rs b/src/tests/hygiene/mod.rs index 9bf89161b24..c950e18da94 100644 --- a/src/tests/hygiene/mod.rs +++ b/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 diff --git a/src/types/mod.rs b/src/types/mod.rs index d1020931d76..bd33e5a3ded 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -153,7 +153,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; From 0270553a5c0b9896846b9ddac54b0f414109cf98 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sun, 22 Sep 2024 16:52:17 +0200 Subject: [PATCH 2/3] fix unintentional `unsafe_code` trigger --- pyo3-macros-backend/src/pymethod.rs | 13 +++++++++---- tests/test_compile_error.rs | 2 ++ tests/ui/forbid_unsafe.rs | 30 +++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) create mode 100644 tests/ui/forbid_unsafe.rs diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index a11e68fe2a1..567bd2f5ac8 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -766,7 +766,14 @@ 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 @@ -790,9 +797,7 @@ pub fn impl_py_getter_def( { #pyo3_path::impl_::pyclass::IsIntoPyObjectRef::<#ty>::VALUE }, { #pyo3_path::impl_::pyclass::IsIntoPyObject::<#ty>::VALUE }, > = unsafe { #pyo3_path::impl_::pyclass::PyClassGetterGenerator::new() }; - #pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Runtime( - || GENERATOR.generate(#python_name, #doc) - ) + #generator } }; diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs index 5ae4e550733..012d759a99d 100644 --- a/tests/test_compile_error.rs +++ b/tests/test_compile_error.rs @@ -54,6 +54,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"); diff --git a/tests/ui/forbid_unsafe.rs b/tests/ui/forbid_unsafe.rs new file mode 100644 index 00000000000..9b62886b650 --- /dev/null +++ b/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() {} From fc06a80958f8dc9c6415c1ba6824a8db66543842 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Mon, 23 Sep 2024 22:09:51 +0200 Subject: [PATCH 3/3] add newsfragment --- newsfragments/4574.fixed.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 newsfragments/4574.fixed.md diff --git a/newsfragments/4574.fixed.md b/newsfragments/4574.fixed.md new file mode 100644 index 00000000000..c996e927289 --- /dev/null +++ b/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. \ No newline at end of file