From 965eb0c43f6ebdb369cb1e269b80f40e43229e55 Mon Sep 17 00:00:00 2001 From: Bas Schoenmaeckers Date: Wed, 19 Aug 2026 17:43:11 +0200 Subject: [PATCH 1/7] Add `PyGcTraversable` derive --- pyo3-macros-backend/src/attributes.rs | 2 + pyo3-macros-backend/src/derive_attributes.rs | 56 +- pyo3-macros-backend/src/lib.rs | 2 + pyo3-macros-backend/src/pygcintegration.rs | 391 ++++++++++++++ pyo3-macros/src/lib.rs | 16 +- src/impl_/pyclass/probes.rs | 6 + src/lib.rs | 3 +- src/prelude.rs | 2 + src/pyclass.rs | 2 +- src/pyclass/gc.rs | 496 ++++++++++++++++++ tests/test_pygcintegration.rs | 208 ++++++++ tests/ui/invalid_pygcintegration_derive.rs | 58 ++ .../ui/invalid_pygcintegration_derive.stderr | 74 +++ 13 files changed, 1283 insertions(+), 33 deletions(-) create mode 100644 pyo3-macros-backend/src/pygcintegration.rs create mode 100644 tests/test_pygcintegration.rs create mode 100644 tests/ui/invalid_pygcintegration_derive.rs create mode 100644 tests/ui/invalid_pygcintegration_derive.stderr diff --git a/pyo3-macros-backend/src/attributes.rs b/pyo3-macros-backend/src/attributes.rs index 9894c463628..75a41e59acb 100644 --- a/pyo3-macros-backend/src/attributes.rs +++ b/pyo3-macros-backend/src/attributes.rs @@ -50,6 +50,7 @@ pub mod kw { syn::custom_keyword!(unsendable); syn::custom_keyword!(weakref); syn::custom_keyword!(generic); + syn::custom_keyword!(gc); syn::custom_keyword!(gil_used); syn::custom_keyword!(warn); syn::custom_keyword!(message); @@ -392,6 +393,7 @@ pub type FromPyWithAttribute = KeywordAttribute; pub type IntoPyWithAttribute = KeywordAttribute; pub type DefaultAttribute = OptionalKeywordAttribute; +pub type GcAttribute = KeywordAttribute; /// For specifying the path to the pyo3 crate. pub type CrateAttribute = KeywordAttribute>; diff --git a/pyo3-macros-backend/src/derive_attributes.rs b/pyo3-macros-backend/src/derive_attributes.rs index 6ec78e17eb0..01c0b2401d7 100644 --- a/pyo3-macros-backend/src/derive_attributes.rs +++ b/pyo3-macros-backend/src/derive_attributes.rs @@ -1,7 +1,7 @@ -use crate::attributes::{ - self, get_pyo3_options, CrateAttribute, DefaultAttribute, FromPyWithAttribute, - IntoPyWithAttribute, RenameAllAttribute, -}; +use crate::attributes::{ + self, get_pyo3_options, CrateAttribute, DefaultAttribute, FromPyWithAttribute, + IntoPyWithAttribute, RenameAllAttribute, +}; use proc_macro2::Span; use syn::parse::{Parse, ParseStream}; use syn::spanned::Spanned; @@ -111,12 +111,12 @@ impl FieldGetter { } } -pub enum FieldAttribute { - Getter(FieldGetter), - FromPyWith(FromPyWithAttribute), - IntoPyWith(IntoPyWithAttribute), - Default(DefaultAttribute), -} +pub enum FieldAttribute { + Getter(FieldGetter), + FromPyWith(FromPyWithAttribute), + IntoPyWith(IntoPyWithAttribute), + Default(DefaultAttribute), +} impl Parse for FieldAttribute { fn parse(input: ParseStream<'_>) -> Result { @@ -159,21 +159,21 @@ impl Parse for FieldAttribute { input.parse().map(Self::FromPyWith) } else if lookahead.peek(attributes::kw::into_py_with) { input.parse().map(FieldAttribute::IntoPyWith) - } else if lookahead.peek(Token![default]) { - input.parse().map(Self::Default) - } else { - Err(lookahead.error()) - } + } else if lookahead.peek(Token![default]) { + input.parse().map(Self::Default) + } else { + Err(lookahead.error()) + } } } #[derive(Clone, Debug, Default)] -pub struct FieldAttributes { - pub getter: Option, - pub from_py_with: Option, - pub into_py_with: Option, - pub default: Option, -} +pub struct FieldAttributes { + pub getter: Option, + pub from_py_with: Option, + pub into_py_with: Option, + pub default: Option, +} impl FieldAttributes { /// Extract the field attributes. @@ -208,10 +208,10 @@ impl FieldAttributes { FieldAttribute::Getter(getter) => { set_option!(getter, "only one of `attribute` or `item` can be provided") } - FieldAttribute::FromPyWith(from_py_with) => set_option!(from_py_with), - FieldAttribute::IntoPyWith(into_py_with) => set_option!(into_py_with), - FieldAttribute::Default(default) => set_option!(default), - } - Ok(()) - } -} + FieldAttribute::FromPyWith(from_py_with) => set_option!(from_py_with), + FieldAttribute::IntoPyWith(into_py_with) => set_option!(into_py_with), + FieldAttribute::Default(default) => set_option!(default), + } + Ok(()) + } +} diff --git a/pyo3-macros-backend/src/lib.rs b/pyo3-macros-backend/src/lib.rs index a90fa73678e..6fb3a2fced6 100644 --- a/pyo3-macros-backend/src/lib.rs +++ b/pyo3-macros-backend/src/lib.rs @@ -13,6 +13,7 @@ mod combine_errors; mod derive_attributes; mod frompyobject; mod intopyobject; +mod pygcintegration; #[cfg(feature = "experimental-inspect")] mod introspection; mod konst; @@ -29,6 +30,7 @@ mod quotes; pub use frompyobject::build_derive_from_pyobject; pub use intopyobject::build_derive_into_pyobject; +pub use pygcintegration::build_derive_py_gc_integration; pub use module::{pymodule_function_impl, pymodule_module_impl, PyModuleOptions}; pub use pyclass::{build_py_class, build_py_enum, PyClassArgs}; pub use pyfunction::{build_py_function, PyFunctionOptions}; diff --git a/pyo3-macros-backend/src/pygcintegration.rs b/pyo3-macros-backend/src/pygcintegration.rs new file mode 100644 index 00000000000..43b5824cdb0 --- /dev/null +++ b/pyo3-macros-backend/src/pygcintegration.rs @@ -0,0 +1,391 @@ +use crate::attributes::{self, get_pyo3_options, GcAttribute}; +use crate::derive_attributes::ContainerAttributes; +use crate::utils::Ctx; +use proc_macro2::{Span, TokenStream}; +use quote::{format_ident, quote}; +use syn::{ + parse::{Parse, ParseStream}, + parse_quote, + punctuated::Punctuated, + spanned::Spanned, + DeriveInput, Fields, Result, Token, +}; + +struct GcField<'a> { + member: syn::Member, + ty: &'a syn::Type, + include: bool, +} + +enum GcFieldAttribute { + Gc(GcAttribute), +} + +impl Parse for GcFieldAttribute { + fn parse(input: ParseStream<'_>) -> Result { + let lookahead = input.lookahead1(); + if lookahead.peek(attributes::kw::gc) { + let attr: GcAttribute = input.parse()?; + Ok(Self::Gc(attr)) + } else { + Err(lookahead.error()) + } + } +} + +fn parse_gc_field<'a>(field: &'a syn::Field, member: syn::Member) -> Result> { + let mut gc = None; + for attr in &field.attrs { + if let Some(options) = get_pyo3_options::(attr)? { + for opt in options { + let GcFieldAttribute::Gc(opt) = opt; + ensure_spanned!( + gc.is_none(), + opt.span() => "`gc` may only be specified once" + ); + gc = Some(opt); + } + } else if attr.path().is_ident("pyo3") { + let _ = attr.parse_args_with(Punctuated::::parse_terminated)?; + } + } + + let include = gc.as_ref().map(|attr| attr.value.value).unwrap_or(true); + Ok(GcField { + member, + ty: &field.ty, + include, + }) +} + +fn fields_for_struct(fields: &Fields) -> Result>> { + match fields { + Fields::Named(named) => named + .named + .iter() + .map(|field| { + let ident = field.ident.as_ref().expect("named field must have ident"); + parse_gc_field(field, syn::Member::Named(ident.clone())) + }) + .collect(), + Fields::Unnamed(unnamed) => unnamed + .unnamed + .iter() + .enumerate() + .map(|(i, field)| parse_gc_field(field, syn::Member::Unnamed(i.into()))) + .collect(), + Fields::Unit => Ok(Vec::new()), + } +} + +fn traverse_stmts_for_struct(fields: &[GcField<'_>], pyo3_path: &crate::utils::PyO3CratePath) -> TokenStream { + let included = fields.iter().filter(|field| field.include).map(|field| { + let member = &field.member; + let ty = field.ty; + quote! { + if <#ty as #pyo3_path::pyclass::PyGcTraversable>::MAY_CONTAIN_CYCLES { + #pyo3_path::pyclass::PyGcTraversable::traverse(&self.#member, visit.clone())?; + } + } + }); + + quote! { + #(#included)* + Ok(()) + } +} + +fn clear_stmts_for_struct(fields: &[GcField<'_>], pyo3_path: &crate::utils::PyO3CratePath) -> TokenStream { + let included = fields.iter().filter(|field| field.include).map(|field| { + let member = &field.member; + let ty = field.ty; + quote! { + if <#ty as #pyo3_path::pyclass::PyGcTraversable>::MAY_CONTAIN_CYCLES { + #pyo3_path::pyclass::PyGcTraversable::clear(&mut self.#member); + } + } + }); + + quote! { + #(#included)* + } +} + +fn cycle_or_expr(fields: &[GcField<'_>], pyo3_path: &crate::utils::PyO3CratePath) -> TokenStream { + let mut included = fields.iter().filter(|field| field.include); + let Some(first) = included.next() else { + return quote!(false); + }; + + let first_ty = first.ty; + let rest = included.map(|field| { + let ty = field.ty; + quote!(|| <#ty as #pyo3_path::pyclass::PyGcTraversable>::MAY_CONTAIN_CYCLES) + }); + + quote!( + <#first_ty as #pyo3_path::pyclass::PyGcTraversable>::MAY_CONTAIN_CYCLES #(#rest)* + ) +} + +fn collect_where_predicates<'a>( + fields: impl Iterator>, + pyo3_path: &crate::utils::PyO3CratePath, +) -> Punctuated { + fields + .filter(|field| field.include) + .map(|field| -> syn::WherePredicate { + let ty = field.ty; + parse_quote!(#ty: #pyo3_path::pyclass::PyGcTraversable) + }) + .collect::>() +} + +fn append_where_predicates( + mut generics: syn::Generics, + predicates: Punctuated, +) -> syn::Generics { + if predicates.is_empty() { + return generics; + } + + let where_clause = generics.make_where_clause(); + where_clause.predicates.extend(predicates); + generics +} + +fn assertion_impl( + fields: &[GcField<'_>], + pyo3_path: &crate::utils::PyO3CratePath, +) -> TokenStream { + let assertions: Vec<_> = fields + .iter() + .filter(|field| !field.include) + .enumerate() + .map(|(i, field)| { + let const_ident = format_ident!("__PYO3_GC_FALSE_ASSERT_{i}", span = Span::call_site()); + let ty = field.ty; + quote! { + const #const_ident: () = { + #[allow(unused_imports, reason = "Probe not used if assertion trips")] + use #pyo3_path::impl_::pyclass::{IsPyGcTraversable, Probe as _}; + assert!( + !IsPyGcTraversable::<#ty>::VALUE, + "`#[pyo3(gc = false)]` may not be used on fields which implement `PyGcTraversable`" + ); + }; + } + }) + .collect(); + + if assertions.is_empty() { + return quote! {}; + } + + quote! { + #(#assertions)* + } +} + +pub fn build_derive_py_gc_integration(tokens: &DeriveInput) -> Result { + let options = ContainerAttributes::from_attrs(&tokens.attrs)?; + ensure_spanned!( + options.transparent.is_none(), + options.transparent.span() => "`transparent` is not supported for `#[derive(PyGcTraversable)]`" + ); + ensure_spanned!( + options.from_item_all.is_none(), + options.from_item_all.span() => "`from_item_all` is not supported for `#[derive(PyGcTraversable)]`" + ); + ensure_spanned!( + options.annotation.is_none(), + options.annotation.span() => "`annotation` is not supported for `#[derive(PyGcTraversable)]`" + ); + ensure_spanned!( + options.rename_all.is_none(), + options.rename_all.span() => "`rename_all` is not supported for `#[derive(PyGcTraversable)]`" + ); + + let ctx = Ctx::new(&options.krate, None); + let pyo3_path = &ctx.pyo3_path; + let ident = &tokens.ident; + + let (fields, traverse_body, clear_body, cycle_expr) = match &tokens.data { + syn::Data::Struct(data) => { + let fields = fields_for_struct(&data.fields)?; + let traverse = traverse_stmts_for_struct(&fields, pyo3_path); + let clear = clear_stmts_for_struct(&fields, pyo3_path); + let cycles = cycle_or_expr(&fields, pyo3_path); + (fields, quote!(#traverse), quote!(#clear), cycles) + } + syn::Data::Enum(data) => { + ensure_spanned!( + !data.variants.is_empty(), + tokens.span() => "cannot derive `PyGcTraversable` for empty enum" + ); + + let mut all_fields = Vec::new(); + let mut traverse_arms = Vec::new(); + let mut clear_arms = Vec::new(); + + for variant in &data.variants { + let variant_ident = &variant.ident; + let variant_fields = fields_for_struct(&variant.fields)?; + all_fields.extend(variant_fields.iter().map(|field| GcField { + member: field.member.clone(), + ty: field.ty, + include: field.include, + })); + + match &variant.fields { + Fields::Named(named) => { + let bindings: Vec<_> = named + .named + .iter() + .enumerate() + .map(|(i, field)| { + let field_ident = field.ident.as_ref().expect("named field"); + let binding = format_ident!("field_{i}"); + quote!(#field_ident: #binding) + }) + .collect(); + + let traverse_stmts = variant_fields + .iter() + .enumerate() + .filter(|(_, field)| field.include) + .map(|(i, field)| { + let binding = format_ident!("field_{i}"); + let ty = field.ty; + quote! { + if <#ty as #pyo3_path::pyclass::PyGcTraversable>::MAY_CONTAIN_CYCLES { + #pyo3_path::pyclass::PyGcTraversable::traverse(#binding, visit.clone())?; + } + } + }); + + let clear_stmts = variant_fields + .iter() + .enumerate() + .filter(|(_, field)| field.include) + .map(|(i, field)| { + let binding = format_ident!("field_{i}"); + let ty = field.ty; + quote! { + if <#ty as #pyo3_path::pyclass::PyGcTraversable>::MAY_CONTAIN_CYCLES { + #pyo3_path::pyclass::PyGcTraversable::clear(#binding); + } + } + }); + + traverse_arms.push(quote! { + Self::#variant_ident { #(#bindings),* } => { + #(#traverse_stmts)* + Ok(()) + } + }); + clear_arms.push(quote! { + Self::#variant_ident { #(#bindings),* } => { + #(#clear_stmts)* + } + }); + } + Fields::Unnamed(unnamed) => { + let bindings: Vec<_> = unnamed + .unnamed + .iter() + .enumerate() + .map(|(i, _)| format_ident!("field_{i}")) + .collect(); + + let traverse_stmts = variant_fields + .iter() + .enumerate() + .filter(|(_, field)| field.include) + .map(|(i, field)| { + let binding = format_ident!("field_{i}"); + let ty = field.ty; + quote! { + if <#ty as #pyo3_path::pyclass::PyGcTraversable>::MAY_CONTAIN_CYCLES { + #pyo3_path::pyclass::PyGcTraversable::traverse(#binding, visit.clone())?; + } + } + }); + + let clear_stmts = variant_fields + .iter() + .enumerate() + .filter(|(_, field)| field.include) + .map(|(i, field)| { + let binding = format_ident!("field_{i}"); + let ty = field.ty; + quote! { + if <#ty as #pyo3_path::pyclass::PyGcTraversable>::MAY_CONTAIN_CYCLES { + #pyo3_path::pyclass::PyGcTraversable::clear(#binding); + } + } + }); + + traverse_arms.push(quote! { + Self::#variant_ident(#(#bindings),*) => { + #(#traverse_stmts)* + Ok(()) + } + }); + clear_arms.push(quote! { + Self::#variant_ident(#(#bindings),*) => { + #(#clear_stmts)* + } + }); + } + Fields::Unit => { + traverse_arms.push(quote!(Self::#variant_ident => Ok(()))); + clear_arms.push(quote!(Self::#variant_ident => {})); + } + } + } + + let cycles = cycle_or_expr(&all_fields, pyo3_path); + ( + all_fields, + quote! { + match self { + #(#traverse_arms),* + } + }, + quote! { + match self { + #(#clear_arms),* + } + }, + cycles, + ) + } + syn::Data::Union(_) => { + bail_spanned!(tokens.span() => "#[derive(PyGcTraversable)] is not supported for unions") + } + }; + + let predicates = collect_where_predicates(fields.iter(), pyo3_path); + let impl_generics = append_where_predicates(tokens.generics.clone(), predicates); + let (impl_generics, ty_generics, where_clause) = impl_generics.split_for_impl(); + + let assertions = assertion_impl(&fields, pyo3_path); + + Ok(quote! { + #[automatically_derived] + unsafe impl #impl_generics #pyo3_path::pyclass::PyGcTraversable for #ident #ty_generics #where_clause { + const MAY_CONTAIN_CYCLES: bool = #cycle_expr; + + fn traverse(&self, visit: #pyo3_path::pyclass::PyVisit<'_>) -> ::std::result::Result<(), #pyo3_path::pyclass::PyTraverseError> { + #traverse_body + } + + fn clear(&mut self) { + #clear_body + } + } + + #assertions + }) +} diff --git a/pyo3-macros/src/lib.rs b/pyo3-macros/src/lib.rs index bba07366b3c..767ff61c76c 100644 --- a/pyo3-macros/src/lib.rs +++ b/pyo3-macros/src/lib.rs @@ -5,9 +5,9 @@ use proc_macro::TokenStream; use proc_macro2::TokenStream as TokenStream2; use pyo3_macros_backend::{ - build_derive_from_pyobject, build_derive_into_pyobject, build_py_class, build_py_enum, - build_py_function, build_py_methods, pymodule_function_impl, pymodule_module_impl, PyClassArgs, - PyClassMethodsType, PyFunctionOptions, PyModuleOptions, + build_derive_from_pyobject, build_derive_into_pyobject, build_derive_py_gc_integration, + build_py_class, build_py_enum, build_py_function, build_py_methods, pymodule_function_impl, + pymodule_module_impl, PyClassArgs, PyClassMethodsType, PyFunctionOptions, PyModuleOptions, }; use quote::quote; use syn::{parse_macro_input, Item}; @@ -188,6 +188,16 @@ pub fn derive_from_py_object(item: TokenStream) -> TokenStream { .into() } +#[proc_macro_derive(PyGcTraversable, attributes(pyo3))] +pub fn derive_py_gc_traversable(item: TokenStream) -> TokenStream { + let ast = parse_macro_input!(item as syn::DeriveInput); + let expanded = build_derive_py_gc_integration(&ast).unwrap_or_compile_error(); + quote!( + #expanded + ) + .into() +} + fn pyclass_impl( attrs: TokenStream, mut ast: syn::ItemStruct, diff --git a/src/impl_/pyclass/probes.rs b/src/impl_/pyclass/probes.rs index 8e32873d751..59f0ac42138 100644 --- a/src/impl_/pyclass/probes.rs +++ b/src/impl_/pyclass/probes.rs @@ -97,6 +97,12 @@ where pub const VALUE: bool = true; } +probe!(IsPyGcTraversable); + +impl IsPyGcTraversable { + pub const VALUE: bool = true; +} + #[cfg(test)] macro_rules! value_of { ($probe:ident, $ty:ty) => {{ diff --git a/src/lib.rs b/src/lib.rs index 3a9f8f36db3..35fc662b69c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -354,7 +354,7 @@ pub use crate::instance::{Borrowed, Bound, BoundObject, Py}; pub use crate::interpreter_lifecycle::with_embedded_python_interpreter; pub use crate::marker::Python; pub use crate::pycell::{PyRef, PyRefMut}; -pub use crate::pyclass::{PyClass, PyClassGuard, PyClassGuardMut}; +pub use crate::pyclass::{PyClass, PyClassGuard, PyClassGuardMut, PyGcOpaque, PyGcTraversable}; pub use crate::pyclass_init::PyClassInitializer; pub use crate::type_object::{PyTypeCheck, PyTypeInfo}; pub use crate::types::PyAny; @@ -456,6 +456,7 @@ pub use crate::conversions::*; #[cfg(feature = "macros")] pub use pyo3_macros::{ pyfunction, pymethods, pymodule, FromPyObject, IntoPyObject, IntoPyObjectRef, + PyGcTraversable, }; /// A proc macro used to expose Rust structs and fieldless enums as Python objects. diff --git a/src/prelude.rs b/src/prelude.rs index 2632d98545e..ed850362eb5 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -13,6 +13,7 @@ pub use crate::err::{PyErr, PyResult}; pub use crate::instance::{Borrowed, Bound, Py}; pub use crate::marker::Python; pub use crate::pycell::{PyRef, PyRefMut}; +pub use crate::pyclass::{PyGcOpaque, PyGcTraversable}; pub use crate::pyclass_init::PyClassInitializer; pub use crate::types::{PyAny, PyModule}; pub use crate::{PyClassGuard, PyClassGuardMut}; @@ -20,6 +21,7 @@ pub use crate::{PyClassGuard, PyClassGuardMut}; #[cfg(feature = "macros")] pub use pyo3_macros::{ pyclass, pyfunction, pymethods, pymodule, FromPyObject, IntoPyObject, IntoPyObjectRef, + PyGcTraversable, }; #[cfg(feature = "macros")] diff --git a/src/pyclass.rs b/src/pyclass.rs index cc58540c7cb..17118f98b24 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -8,7 +8,7 @@ mod guard; pub(crate) use self::create_type_object::{create_type_object, PyClassTypeObject}; -pub use self::gc::{PyTraverseError, PyVisit}; +pub use self::gc::{PyGcOpaque, PyGcTraversable, PyTraverseError, PyVisit}; pub use self::guard::{ PyClassGuard, PyClassGuardError, PyClassGuardMap, PyClassGuardMut, PyClassGuardMutError, PyClassGuardMutSuper, diff --git a/src/pyclass/gc.rs b/src/pyclass/gc.rs index 63d38230808..8c8b85e4cdc 100644 --- a/src/pyclass/gc.rs +++ b/src/pyclass/gc.rs @@ -1,11 +1,507 @@ use core::{ + ffi::CStr, ffi::{c_int, c_void}, marker::PhantomData, num::NonZero, + ops::{Deref, DerefMut}, +}; + +use alloc::{ + boxed::Box, + collections::{BTreeMap, BTreeSet, BinaryHeap, LinkedList, VecDeque}, + ffi::CString, + string::String, + vec::Vec, +}; +use std::{ + collections::{HashMap, HashSet}, + ffi::{OsStr, OsString}, + hash::{BuildHasher, Hash}, + path::{Path, PathBuf}, + sync::OnceLock, }; use crate::{ffi, Py}; +/// Trait describing how values participate in Python's cyclic garbage collector. +/// +/// # Safety +/// +/// Implementations must not execute arbitrary Python code from `traverse`. +pub unsafe trait PyGcTraversable { + /// Whether this type may hold Python object references which can participate in cycles. + const MAY_CONTAIN_CYCLES: bool; + + /// Visit all Python objects referenced by this value. + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>; + + /// Clear references held by this value to help break cycles. + fn clear(&mut self); +} + +macro_rules! impl_py_gc_no_cycles { + ($($ty:ty),* $(,)?) => { + $( + unsafe impl PyGcTraversable for $ty { + const MAY_CONTAIN_CYCLES: bool = false; + + #[inline] + fn traverse(&self, _visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + Ok(()) + } + + #[inline] + fn clear(&mut self) {} + } + )* + }; +} + +impl_py_gc_no_cycles!( + (), + bool, + char, + i8, + i16, + i32, + i64, + i128, + isize, + u8, + u16, + u32, + u64, + u128, + usize, + f32, + f64, + str, + String, + CString, + CStr, + Path, + PathBuf, + OsStr, + OsString, +); + +unsafe impl PyGcTraversable for &T { + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; + + #[inline] + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if T::MAY_CONTAIN_CYCLES { + (**self).traverse(visit) + } else { + Ok(()) + } + } + + #[inline] + fn clear(&mut self) {} +} + +unsafe impl PyGcTraversable for &mut T { + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; + + #[inline] + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if T::MAY_CONTAIN_CYCLES { + (**self).traverse(visit) + } else { + Ok(()) + } + } + + #[inline] + fn clear(&mut self) { + if T::MAY_CONTAIN_CYCLES { + (**self).clear(); + } + } +} + +unsafe impl PyGcTraversable for PhantomData { + const MAY_CONTAIN_CYCLES: bool = false; + + #[inline] + fn traverse(&self, _visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + Ok(()) + } + + #[inline] + fn clear(&mut self) {} +} + +unsafe impl PyGcTraversable for Option { + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if T::MAY_CONTAIN_CYCLES { + if let Some(value) = self { + value.traverse(visit)?; + } + } + Ok(()) + } + + fn clear(&mut self) { + if T::MAY_CONTAIN_CYCLES { + *self = None; + } + } +} + +unsafe impl PyGcTraversable for Vec { + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if T::MAY_CONTAIN_CYCLES { + for item in self { + item.traverse(visit.clone())?; + } + } + Ok(()) + } + + fn clear(&mut self) { + if T::MAY_CONTAIN_CYCLES { + Vec::clear(self); + } + } +} + +unsafe impl PyGcTraversable for VecDeque { + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if T::MAY_CONTAIN_CYCLES { + for item in self { + item.traverse(visit.clone())?; + } + } + Ok(()) + } + + fn clear(&mut self) { + if T::MAY_CONTAIN_CYCLES { + VecDeque::clear(self); + } + } +} + +unsafe impl PyGcTraversable for LinkedList { + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if T::MAY_CONTAIN_CYCLES { + for item in self { + item.traverse(visit.clone())?; + } + } + Ok(()) + } + + fn clear(&mut self) { + if T::MAY_CONTAIN_CYCLES { + LinkedList::clear(self); + } + } +} + +unsafe impl PyGcTraversable for BinaryHeap { + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if T::MAY_CONTAIN_CYCLES { + for item in self { + item.traverse(visit.clone())?; + } + } + Ok(()) + } + + fn clear(&mut self) { + if T::MAY_CONTAIN_CYCLES { + BinaryHeap::clear(self); + } + } +} + +unsafe impl PyGcTraversable for HashMap +where + K: PyGcTraversable + Eq + Hash, + V: PyGcTraversable, + S: BuildHasher, +{ + const MAY_CONTAIN_CYCLES: bool = K::MAY_CONTAIN_CYCLES || V::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if K::MAY_CONTAIN_CYCLES || V::MAY_CONTAIN_CYCLES { + for (key, value) in self { + if K::MAY_CONTAIN_CYCLES { + key.traverse(visit.clone())?; + } + if V::MAY_CONTAIN_CYCLES { + value.traverse(visit.clone())?; + } + } + } + Ok(()) + } + + fn clear(&mut self) { + if K::MAY_CONTAIN_CYCLES || V::MAY_CONTAIN_CYCLES { + HashMap::clear(self); + } + } +} + +unsafe impl PyGcTraversable for HashSet +where + T: PyGcTraversable + Eq + Hash, + S: BuildHasher, +{ + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if T::MAY_CONTAIN_CYCLES { + for item in self { + item.traverse(visit.clone())?; + } + } + Ok(()) + } + + fn clear(&mut self) { + if T::MAY_CONTAIN_CYCLES { + HashSet::clear(self); + } + } +} + +unsafe impl PyGcTraversable for BTreeMap +where + K: PyGcTraversable + Ord, + V: PyGcTraversable, +{ + const MAY_CONTAIN_CYCLES: bool = K::MAY_CONTAIN_CYCLES || V::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if K::MAY_CONTAIN_CYCLES || V::MAY_CONTAIN_CYCLES { + for (key, value) in self { + if K::MAY_CONTAIN_CYCLES { + key.traverse(visit.clone())?; + } + if V::MAY_CONTAIN_CYCLES { + value.traverse(visit.clone())?; + } + } + } + Ok(()) + } + + fn clear(&mut self) { + if K::MAY_CONTAIN_CYCLES || V::MAY_CONTAIN_CYCLES { + BTreeMap::clear(self); + } + } +} + +unsafe impl PyGcTraversable for BTreeSet { + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if T::MAY_CONTAIN_CYCLES { + for item in self { + item.traverse(visit.clone())?; + } + } + Ok(()) + } + + fn clear(&mut self) { + if T::MAY_CONTAIN_CYCLES { + BTreeSet::clear(self); + } + } +} + +unsafe impl PyGcTraversable for OnceLock { + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if T::MAY_CONTAIN_CYCLES { + if let Some(value) = self.get() { + value.traverse(visit)?; + } + } + Ok(()) + } + + fn clear(&mut self) { + if T::MAY_CONTAIN_CYCLES { + let _ = self.take(); + } + } +} + +unsafe impl PyGcTraversable for Result +where + T: PyGcTraversable, + E: PyGcTraversable, +{ + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES || E::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if T::MAY_CONTAIN_CYCLES || E::MAY_CONTAIN_CYCLES { + match self { + Ok(value) if T::MAY_CONTAIN_CYCLES => value.traverse(visit)?, + Err(error) if E::MAY_CONTAIN_CYCLES => error.traverse(visit)?, + _ => {} + } + } + Ok(()) + } + + fn clear(&mut self) { + if T::MAY_CONTAIN_CYCLES || E::MAY_CONTAIN_CYCLES { + match self { + Ok(value) if T::MAY_CONTAIN_CYCLES => value.clear(), + Err(error) if E::MAY_CONTAIN_CYCLES => error.clear(), + _ => {} + } + } + } +} + +unsafe impl PyGcTraversable for [T; N] { + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if T::MAY_CONTAIN_CYCLES { + for item in self { + item.traverse(visit.clone())?; + } + } + Ok(()) + } + + fn clear(&mut self) { + if T::MAY_CONTAIN_CYCLES { + for item in self { + item.clear(); + } + } + } +} + +unsafe impl PyGcTraversable for [T] { + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if T::MAY_CONTAIN_CYCLES { + for item in self { + item.traverse(visit.clone())?; + } + } + Ok(()) + } + + fn clear(&mut self) { + if T::MAY_CONTAIN_CYCLES { + for item in self { + item.clear(); + } + } + } +} + +unsafe impl PyGcTraversable for Box { + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if T::MAY_CONTAIN_CYCLES { + (**self).traverse(visit)?; + } + Ok(()) + } + + fn clear(&mut self) { + if T::MAY_CONTAIN_CYCLES { + (**self).clear(); + } + } +} + +unsafe impl PyGcTraversable for Py { + const MAY_CONTAIN_CYCLES: bool = true; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + visit.call(self) + } + + fn clear(&mut self) {} +} + +/// Wrapper to explicitly opt out of GC traversal for a type. +/// +/// This is useful for intentional recursion breakpoints where traversing a +/// reference would recurse indefinitely. Only use this when the wrapped value +/// is known to be traversed through another path. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[repr(transparent)] +pub struct PyGcOpaque(T); + +impl PyGcOpaque { + /// Wrap `inner` as GC-opaque. + #[inline] + pub const fn new(inner: T) -> Self { + Self(inner) + } + + /// Consume this wrapper and return the wrapped value. + #[inline] + pub fn into_inner(self) -> T { + self.0 + } +} + +impl Deref for PyGcOpaque { + type Target = T; + + #[inline] + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for PyGcOpaque { + #[inline] + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + +impl From for PyGcOpaque { + #[inline] + fn from(value: T) -> Self { + Self(value) + } +} + +unsafe impl PyGcTraversable for PyGcOpaque { + const MAY_CONTAIN_CYCLES: bool = false; + + #[inline] + fn traverse(&self, _visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + Ok(()) + } + + #[inline] + fn clear(&mut self) {} +} + /// Error returned by a `__traverse__` visitor implementation. #[repr(transparent)] pub struct PyTraverseError(NonZero); diff --git a/tests/test_pygcintegration.rs b/tests/test_pygcintegration.rs new file mode 100644 index 00000000000..fdf85e4de07 --- /dev/null +++ b/tests/test_pygcintegration.rs @@ -0,0 +1,208 @@ +#![cfg(feature = "macros")] + +use pyo3::prelude::*; +use pyo3::types::PyDict; +use pyo3::{PyTraverseError, PyVisit}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet, LinkedList, VecDeque}; +use std::marker::PhantomData; +use std::sync::OnceLock; + +struct NotTraversable { + value: i32, +} + +#[derive(PyGcTraversable, PartialEq, Eq, PartialOrd, Ord)] +struct Leaf { + value: i32, +} + +impl std::hash::Hash for Leaf { + fn hash(&self, state: &mut H) { + self.value.hash(state); + } +} + +#[derive(PyGcTraversable)] +struct Branch { + left: Option, + right: Vec, +} + +#[derive(PyGcTraversable)] +struct BranchWithIgnoredField { + tracked: Option, + #[pyo3(gc = false)] + ignored: NotTraversable, +} + +#[derive(PyGcTraversable)] +struct Wrappers { + map: BTreeMap, + set: BTreeSet, + lock: OnceLock, + marker: PhantomData, + result: Result, + array: [Leaf; 2], + hash_map: HashMap, + hash_set: HashSet, + vec_deque: VecDeque, + linked_list: LinkedList, +} + +#[derive(PyGcTraversable)] +struct OpaqueWrapper { + hidden: PyGcOpaque>, +} + +#[pyclass] +#[derive(PyGcTraversable)] +struct TraversedByPyClass { + field: Py, +} + +#[pymethods] +impl TraversedByPyClass { + fn __traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + PyGcTraversable::traverse(self, visit) + } + + fn __clear__(&mut self) { + PyGcTraversable::clear(self); + } +} + +#[derive(PyGcTraversable)] +enum Node { + #[allow(dead_code)] + Unit, + #[allow(dead_code)] + Tuple(Option, i32), + #[allow(dead_code)] + Struct { + left: Option, + right: Vec, + }, +} + +#[test] +fn may_contain_cycles_structs_and_enums() { + assert!(!Leaf::MAY_CONTAIN_CYCLES); + assert!(!Branch::MAY_CONTAIN_CYCLES); + assert!(!BranchWithIgnoredField::MAY_CONTAIN_CYCLES); + assert!(!Node::MAY_CONTAIN_CYCLES); +} + +#[test] +fn clear_recursively_clears_supported_types() { + let mut branch = Branch { + left: Some(Leaf { value: 1 }), + right: vec![Leaf { value: 2 }], + }; + + PyGcTraversable::clear(&mut branch); + assert!(branch.left.is_some()); + assert_eq!(branch.right.len(), 1); + + let mut maybe_branch = Some(branch); + PyGcTraversable::clear(&mut maybe_branch); + assert!(maybe_branch.is_some()); +} + +#[test] +fn clear_ignored_field_is_noop() { + let mut value = BranchWithIgnoredField { + tracked: Some(Leaf { value: 5 }), + ignored: NotTraversable { value: 10 }, + }; + + PyGcTraversable::clear(&mut value); + assert_eq!(value.ignored.value, 10); + assert!(value.tracked.is_some()); +} + +#[test] +fn wrappers_compile_and_clear() { + let mut map = BTreeMap::new(); + map.insert(Leaf { value: 1 }, Leaf { value: 2 }); + + let mut set = BTreeSet::new(); + let _ = set.insert(Leaf { value: 3 }); + + let lock = OnceLock::new(); + let _ = lock.set(Leaf { value: 4 }); + + let mut hash_map = HashMap::new(); + hash_map.insert(Leaf { value: 10 }, Leaf { value: 11 }); + + let mut hash_set = HashSet::new(); + let _ = hash_set.insert(Leaf { value: 12 }); + + let mut vec_deque = VecDeque::new(); + vec_deque.push_back(Leaf { value: 13 }); + + let mut linked_list = LinkedList::new(); + linked_list.push_back(Leaf { value: 14 }); + + let mut wrappers = Wrappers { + map, + set, + lock, + marker: PhantomData, + result: Ok(Leaf { value: 7 }), + array: [Leaf { value: 8 }, Leaf { value: 9 }], + hash_map, + hash_set, + vec_deque, + linked_list, + }; + + PyGcTraversable::clear(&mut wrappers); + assert_eq!(wrappers.map.len(), 1); + assert_eq!(wrappers.set.len(), 1); + assert!(wrappers.lock.get().is_some()); + assert_eq!(wrappers.hash_map.len(), 1); + assert_eq!(wrappers.hash_set.len(), 1); + assert_eq!(wrappers.vec_deque.len(), 1); + assert_eq!(wrappers.linked_list.len(), 1); +} + +#[test] +fn opaque_wrapper_breaks_traversal_chain() { + assert!(!OpaqueWrapper::MAY_CONTAIN_CYCLES); + + Python::attach(|py| { + let obj = py.None(); + let mut value = OpaqueWrapper { + hidden: PyGcOpaque::new(obj), + }; + + let ptr_before = value.hidden.as_ptr(); + PyGcTraversable::clear(&mut value); + assert_eq!(ptr_before, value.hidden.as_ptr()); + }); +} + +#[test] +fn direct_py_field_is_traversed() { + Python::attach(|py| { + let inner = py.None(); + let value = Py::new(py, TraversedByPyClass { field: inner.clone_ref(py) }).unwrap(); + + let locals = PyDict::new(py); + locals.set_item("gc", py.import("gc").unwrap()).unwrap(); + locals.set_item("obj", value.bind(py)).unwrap(); + locals.set_item("inner", inner.bind(py)).unwrap(); + + py.run( + c"found = False +for r in gc.get_referents(obj): + if r is inner: + found = True + break +assert found", + None, + Some(&locals), + ) + .unwrap(); + }); +} diff --git a/tests/ui/invalid_pygcintegration_derive.rs b/tests/ui/invalid_pygcintegration_derive.rs new file mode 100644 index 00000000000..281949be3f6 --- /dev/null +++ b/tests/ui/invalid_pygcintegration_derive.rs @@ -0,0 +1,58 @@ +use pyo3::prelude::*; + +struct NotTraversable; + +struct Traversable; + +unsafe impl PyGcTraversable for Traversable { + const MAY_CONTAIN_CYCLES: bool = false; + + fn traverse(&self, _visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + Ok(()) + } + + fn clear(&mut self) {} +} + +#[derive(PyGcTraversable)] +//~^ ERROR: the trait bound `NotTraversable: pyo3::PyGcTraversable` is not satisfied +struct MissingFieldImpl { + field: NotTraversable, +} + +#[derive(PyGcTraversable)] +//~^ ERROR: the trait bound `NotTraversable: pyo3::PyGcTraversable` is not satisfied +struct InvalidGcTrue { + #[pyo3(gc = true)] + field: NotTraversable, +} + +#[derive(PyGcTraversable)] +struct InvalidGcValue { + #[pyo3(gc = "false")] + //~^ ERROR: expected boolean literal + field: NotTraversable, +} + +#[derive(PyGcTraversable)] +//~^ ERROR: evaluation panicked: `#[pyo3(gc = false)]` may not be used on fields which implement `PyGcTraversable` +struct InvalidGcFalseOnIntegrated { + #[pyo3(gc = false)] + field: Traversable, +} + +#[derive(PyGcTraversable)] +struct DuplicateGc { + #[pyo3(gc = false, gc = false)] + //~^ ERROR: `gc` may only be specified once + field: NotTraversable, +} + +#[derive(PyGcTraversable)] +struct UnsupportedFieldAttribute { + #[pyo3(attribute)] + //~^ ERROR: expected `gc` + field: NotTraversable, +} + +fn main() {} diff --git a/tests/ui/invalid_pygcintegration_derive.stderr b/tests/ui/invalid_pygcintegration_derive.stderr new file mode 100644 index 00000000000..1ecb660cf5f --- /dev/null +++ b/tests/ui/invalid_pygcintegration_derive.stderr @@ -0,0 +1,74 @@ +error: expected boolean literal + --> tests/ui/invalid_pygcintegration_derive.rs:32:17 + | +32 | #[pyo3(gc = "false")] + | ^^^^^^^ + +error: `gc` may only be specified once + --> tests/ui/invalid_pygcintegration_derive.rs:46:24 + | +46 | #[pyo3(gc = false, gc = false)] + | ^^ + +error: expected `gc` + --> tests/ui/invalid_pygcintegration_derive.rs:53:12 + | +53 | #[pyo3(attribute)] + | ^^^^^^^^^ + +error[E0277]: the trait bound `NotTraversable: pyo3::PyGcTraversable` is not satisfied + --> tests/ui/invalid_pygcintegration_derive.rs:17:10 + | +17 | #[derive(PyGcTraversable)] + | ^^^^^^^^^^^^^^^ unsatisfied trait bound + | +help: the trait `pyo3::PyGcTraversable` is not implemented for `NotTraversable` + --> tests/ui/invalid_pygcintegration_derive.rs:3:1 + | + 3 | struct NotTraversable; + | ^^^^^^^^^^^^^^^^^^^^^ + = help: the following other types implement trait `pyo3::PyGcTraversable`: + &T + &mut T + () + BTreeMap + BTreeSet + BinaryHeap + Box + CStr + and $N others + = note: this error originates in the derive macro `PyGcTraversable` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: the trait bound `NotTraversable: pyo3::PyGcTraversable` is not satisfied + --> tests/ui/invalid_pygcintegration_derive.rs:23:10 + | +23 | #[derive(PyGcTraversable)] + | ^^^^^^^^^^^^^^^ unsatisfied trait bound + | +help: the trait `pyo3::PyGcTraversable` is not implemented for `NotTraversable` + --> tests/ui/invalid_pygcintegration_derive.rs:3:1 + | + 3 | struct NotTraversable; + | ^^^^^^^^^^^^^^^^^^^^^ + = help: the following other types implement trait `pyo3::PyGcTraversable`: + &T + &mut T + () + BTreeMap + BTreeSet + BinaryHeap + Box + CStr + and $N others + = note: this error originates in the derive macro `PyGcTraversable` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0080]: evaluation panicked: `#[pyo3(gc = false)]` may not be used on fields which implement `PyGcTraversable` + --> tests/ui/invalid_pygcintegration_derive.rs:37:10 + | +37 | #[derive(PyGcTraversable)] + | ^^^^^^^^^^^^^^^ evaluation of `__PYO3_GC_FALSE_ASSERT_0` failed here + +error: aborting due to 6 previous errors + +Some errors have detailed explanations: E0080, E0277. +For more information about an error, try `rustc --explain E0080`. From 99fb5b77503386e9336af56879e4a690f59e4ef2 Mon Sep 17 00:00:00 2001 From: Bas Schoenmaeckers Date: Wed, 19 Aug 2026 18:47:59 +0200 Subject: [PATCH 2/7] Fix fmt/clippy --- pyo3-macros-backend/src/lib.rs | 4 ++-- pyo3-macros-backend/src/pygcintegration.rs | 18 +++++++++------- src/lib.rs | 3 +-- src/pyclass/gc.rs | 24 +++++++++++++++++++++- tests/test_pygcintegration.rs | 20 ++++++++++++------ 5 files changed, 51 insertions(+), 18 deletions(-) diff --git a/pyo3-macros-backend/src/lib.rs b/pyo3-macros-backend/src/lib.rs index 6fb3a2fced6..00ca00fba8c 100644 --- a/pyo3-macros-backend/src/lib.rs +++ b/pyo3-macros-backend/src/lib.rs @@ -13,7 +13,6 @@ mod combine_errors; mod derive_attributes; mod frompyobject; mod intopyobject; -mod pygcintegration; #[cfg(feature = "experimental-inspect")] mod introspection; mod konst; @@ -24,15 +23,16 @@ mod params; mod py_expr; mod pyclass; mod pyfunction; +mod pygcintegration; mod pyimpl; mod pymethod; mod quotes; pub use frompyobject::build_derive_from_pyobject; pub use intopyobject::build_derive_into_pyobject; -pub use pygcintegration::build_derive_py_gc_integration; pub use module::{pymodule_function_impl, pymodule_module_impl, PyModuleOptions}; pub use pyclass::{build_py_class, build_py_enum, PyClassArgs}; pub use pyfunction::{build_py_function, PyFunctionOptions}; +pub use pygcintegration::build_derive_py_gc_integration; pub use pyimpl::{build_py_methods, PyClassMethodsType}; pub use utils::get_doc; diff --git a/pyo3-macros-backend/src/pygcintegration.rs b/pyo3-macros-backend/src/pygcintegration.rs index 43b5824cdb0..1529dfb14e2 100644 --- a/pyo3-macros-backend/src/pygcintegration.rs +++ b/pyo3-macros-backend/src/pygcintegration.rs @@ -46,7 +46,8 @@ fn parse_gc_field<'a>(field: &'a syn::Field, member: syn::Member) -> Result::parse_terminated)?; + let _ = + attr.parse_args_with(Punctuated::::parse_terminated)?; } } @@ -78,7 +79,10 @@ fn fields_for_struct(fields: &Fields) -> Result>> { } } -fn traverse_stmts_for_struct(fields: &[GcField<'_>], pyo3_path: &crate::utils::PyO3CratePath) -> TokenStream { +fn traverse_stmts_for_struct( + fields: &[GcField<'_>], + pyo3_path: &crate::utils::PyO3CratePath, +) -> TokenStream { let included = fields.iter().filter(|field| field.include).map(|field| { let member = &field.member; let ty = field.ty; @@ -95,7 +99,10 @@ fn traverse_stmts_for_struct(fields: &[GcField<'_>], pyo3_path: &crate::utils::P } } -fn clear_stmts_for_struct(fields: &[GcField<'_>], pyo3_path: &crate::utils::PyO3CratePath) -> TokenStream { +fn clear_stmts_for_struct( + fields: &[GcField<'_>], + pyo3_path: &crate::utils::PyO3CratePath, +) -> TokenStream { let included = fields.iter().filter(|field| field.include).map(|field| { let member = &field.member; let ty = field.ty; @@ -154,10 +161,7 @@ fn append_where_predicates( generics } -fn assertion_impl( - fields: &[GcField<'_>], - pyo3_path: &crate::utils::PyO3CratePath, -) -> TokenStream { +fn assertion_impl(fields: &[GcField<'_>], pyo3_path: &crate::utils::PyO3CratePath) -> TokenStream { let assertions: Vec<_> = fields .iter() .filter(|field| !field.include) diff --git a/src/lib.rs b/src/lib.rs index 35fc662b69c..fc7ece8a189 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -455,8 +455,7 @@ pub use crate::conversions::*; #[cfg(feature = "macros")] pub use pyo3_macros::{ - pyfunction, pymethods, pymodule, FromPyObject, IntoPyObject, IntoPyObjectRef, - PyGcTraversable, + pyfunction, pymethods, pymodule, FromPyObject, IntoPyObject, IntoPyObjectRef, PyGcTraversable, }; /// A proc macro used to expose Rust structs and fieldless enums as Python objects. diff --git a/src/pyclass/gc.rs b/src/pyclass/gc.rs index 8c8b85e4cdc..6184a1f0d64 100644 --- a/src/pyclass/gc.rs +++ b/src/pyclass/gc.rs @@ -1,6 +1,7 @@ use core::{ ffi::CStr, ffi::{c_int, c_void}, + hash::{BuildHasher, Hash}, marker::PhantomData, num::NonZero, ops::{Deref, DerefMut}, @@ -16,7 +17,6 @@ use alloc::{ use std::{ collections::{HashMap, HashSet}, ffi::{OsStr, OsString}, - hash::{BuildHasher, Hash}, path::{Path, PathBuf}, sync::OnceLock, }; @@ -42,6 +42,8 @@ pub unsafe trait PyGcTraversable { macro_rules! impl_py_gc_no_cycles { ($($ty:ty),* $(,)?) => { $( + // SAFETY: These types contain no Python object references and therefore + // can safely report no cycles and perform no clearing. unsafe impl PyGcTraversable for $ty { const MAY_CONTAIN_CYCLES: bool = false; @@ -85,6 +87,8 @@ impl_py_gc_no_cycles!( OsString, ); +// SAFETY: Shared references do not own data; forwarding traversal is correct and +// clear is a no-op because `&T` cannot clear through immutable access. unsafe impl PyGcTraversable for &T { const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; @@ -101,6 +105,7 @@ unsafe impl PyGcTraversable for &T { fn clear(&mut self) {} } +// SAFETY: Mutable references can forward both traversal and clearing to `T`. unsafe impl PyGcTraversable for &mut T { const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; @@ -121,6 +126,7 @@ unsafe impl PyGcTraversable for &mut T { } } +// SAFETY: `PhantomData` stores no runtime data and cannot reference Python objects. unsafe impl PyGcTraversable for PhantomData { const MAY_CONTAIN_CYCLES: bool = false; @@ -133,6 +139,7 @@ unsafe impl PyGcTraversable for PhantomData { fn clear(&mut self) {} } +// SAFETY: `Option` contains at most one `T`; delegating to contained value is sound. unsafe impl PyGcTraversable for Option { const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; @@ -152,6 +159,7 @@ unsafe impl PyGcTraversable for Option { } } +// SAFETY: `Vec` owns zero or more `T`; visiting / clearing each element is sound. unsafe impl PyGcTraversable for Vec { const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; @@ -171,6 +179,7 @@ unsafe impl PyGcTraversable for Vec { } } +// SAFETY: `VecDeque` owns zero or more `T`; visiting / clearing each element is sound. unsafe impl PyGcTraversable for VecDeque { const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; @@ -190,6 +199,7 @@ unsafe impl PyGcTraversable for VecDeque { } } +// SAFETY: `LinkedList` owns zero or more `T`; visiting / clearing each element is sound. unsafe impl PyGcTraversable for LinkedList { const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; @@ -209,6 +219,7 @@ unsafe impl PyGcTraversable for LinkedList { } } +// SAFETY: `BinaryHeap` owns zero or more `T`; visiting / clearing each element is sound. unsafe impl PyGcTraversable for BinaryHeap { const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; @@ -228,6 +239,7 @@ unsafe impl PyGcTraversable for BinaryHeap { } } +// SAFETY: `HashMap` owns keys and values; visiting / clearing entries is sound. unsafe impl PyGcTraversable for HashMap where K: PyGcTraversable + Eq + Hash, @@ -257,6 +269,7 @@ where } } +// SAFETY: `HashSet` owns zero or more `T`; visiting / clearing each element is sound. unsafe impl PyGcTraversable for HashSet where T: PyGcTraversable + Eq + Hash, @@ -280,6 +293,7 @@ where } } +// SAFETY: `BTreeMap` owns keys and values; visiting / clearing entries is sound. unsafe impl PyGcTraversable for BTreeMap where K: PyGcTraversable + Ord, @@ -308,6 +322,7 @@ where } } +// SAFETY: `BTreeSet` owns zero or more `T`; visiting / clearing each element is sound. unsafe impl PyGcTraversable for BTreeSet { const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; @@ -327,6 +342,7 @@ unsafe impl PyGcTraversable for BTreeSet { } } +// SAFETY: `OnceLock` owns at most one initialized `T`; delegating if present is sound. unsafe impl PyGcTraversable for OnceLock { const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; @@ -346,6 +362,7 @@ unsafe impl PyGcTraversable for OnceLock { } } +// SAFETY: `Result` owns either `T` or `E`; delegating to active variant is sound. unsafe impl PyGcTraversable for Result where T: PyGcTraversable, @@ -375,6 +392,7 @@ where } } +// SAFETY: Arrays own all elements; visiting / clearing each element is sound. unsafe impl PyGcTraversable for [T; N] { const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; @@ -396,6 +414,7 @@ unsafe impl PyGcTraversable for [T; N] { } } +// SAFETY: Slices reference elements; traversal and clear forwarding per element is sound. unsafe impl PyGcTraversable for [T] { const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; @@ -417,6 +436,7 @@ unsafe impl PyGcTraversable for [T] { } } +// SAFETY: `Box` uniquely owns one `T`; delegating traversal and clear is sound. unsafe impl PyGcTraversable for Box { const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; @@ -434,6 +454,7 @@ unsafe impl PyGcTraversable for Box { } } +// SAFETY: `Py` is a strong reference to a Python object and must always be visited. unsafe impl PyGcTraversable for Py { const MAY_CONTAIN_CYCLES: bool = true; @@ -490,6 +511,7 @@ impl From for PyGcOpaque { } } +// SAFETY: `PyGcOpaque` intentionally opts out of traversal and clear by contract. unsafe impl PyGcTraversable for PyGcOpaque { const MAY_CONTAIN_CYCLES: bool = false; diff --git a/tests/test_pygcintegration.rs b/tests/test_pygcintegration.rs index fdf85e4de07..e96c22301fb 100644 --- a/tests/test_pygcintegration.rs +++ b/tests/test_pygcintegration.rs @@ -86,10 +86,12 @@ enum Node { #[test] fn may_contain_cycles_structs_and_enums() { - assert!(!Leaf::MAY_CONTAIN_CYCLES); - assert!(!Branch::MAY_CONTAIN_CYCLES); - assert!(!BranchWithIgnoredField::MAY_CONTAIN_CYCLES); - assert!(!Node::MAY_CONTAIN_CYCLES); + const { + assert!(!Leaf::MAY_CONTAIN_CYCLES); + assert!(!Branch::MAY_CONTAIN_CYCLES); + assert!(!BranchWithIgnoredField::MAY_CONTAIN_CYCLES); + assert!(!Node::MAY_CONTAIN_CYCLES); + } } #[test] @@ -168,7 +170,7 @@ fn wrappers_compile_and_clear() { #[test] fn opaque_wrapper_breaks_traversal_chain() { - assert!(!OpaqueWrapper::MAY_CONTAIN_CYCLES); + const { assert!(!OpaqueWrapper::MAY_CONTAIN_CYCLES) }; Python::attach(|py| { let obj = py.None(); @@ -186,7 +188,13 @@ fn opaque_wrapper_breaks_traversal_chain() { fn direct_py_field_is_traversed() { Python::attach(|py| { let inner = py.None(); - let value = Py::new(py, TraversedByPyClass { field: inner.clone_ref(py) }).unwrap(); + let value = Py::new( + py, + TraversedByPyClass { + field: inner.clone_ref(py), + }, + ) + .unwrap(); let locals = PyDict::new(py); locals.set_item("gc", py.import("gc").unwrap()).unwrap(); From b67e5cd1591e6dcef396ae7394cab1ef31d03a1e Mon Sep 17 00:00:00 2001 From: Bas Schoenmaeckers Date: Wed, 19 Aug 2026 18:49:11 +0200 Subject: [PATCH 3/7] Add changelog --- newsfragments/6330.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/6330.added.md diff --git a/newsfragments/6330.added.md b/newsfragments/6330.added.md new file mode 100644 index 00000000000..7bc7dacef41 --- /dev/null +++ b/newsfragments/6330.added.md @@ -0,0 +1 @@ +Add `PyGcTraversable` and `#[derive(PyGcTraversable)]` for opt-in GC traversal derivation, including field-level `#[pyo3(gc = false)]` support and a `PyGcOpaque` wrapper for explicit traversal opt-out in recursion breakpoints. From c5538394ade21369b78f7d65a1029726df0bdfa7 Mon Sep 17 00:00:00 2001 From: Bas Schoenmaeckers Date: Wed, 19 Aug 2026 18:59:00 +0200 Subject: [PATCH 4/7] Fix ci --- Cargo.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 7af1eed7d14..b84835a5885 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -346,6 +346,10 @@ required-features = ["macros"] name = "test_gc" required-features = ["macros"] +[[test]] +name = "test_pygcintegration" +required-features = ["macros"] + [[test]] name = "test_getter_setter" required-features = ["macros"] From dc228de27d2b72aaf0a8f6e136d054ee6b3c9564 Mon Sep 17 00:00:00 2001 From: Bas Schoenmaeckers Date: Thu, 20 Aug 2026 09:38:41 +0200 Subject: [PATCH 5/7] Add `Cow` impl for `PyGcTraversable` --- src/pyclass/gc.rs | 48 +++++++++++++++++++++++++++-------- tests/test_pygcintegration.rs | 9 ++++++- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/src/pyclass/gc.rs b/src/pyclass/gc.rs index 6184a1f0d64..0a54a6531ba 100644 --- a/src/pyclass/gc.rs +++ b/src/pyclass/gc.rs @@ -1,3 +1,12 @@ +use crate::{ffi, Py}; +use alloc::{ + borrow::{Cow, ToOwned}, + boxed::Box, + collections::{BTreeMap, BTreeSet, BinaryHeap, LinkedList, VecDeque}, + ffi::CString, + string::String, + vec::Vec, +}; use core::{ ffi::CStr, ffi::{c_int, c_void}, @@ -6,14 +15,6 @@ use core::{ num::NonZero, ops::{Deref, DerefMut}, }; - -use alloc::{ - boxed::Box, - collections::{BTreeMap, BTreeSet, BinaryHeap, LinkedList, VecDeque}, - ffi::CString, - string::String, - vec::Vec, -}; use std::{ collections::{HashMap, HashSet}, ffi::{OsStr, OsString}, @@ -21,8 +22,6 @@ use std::{ sync::OnceLock, }; -use crate::{ffi, Py}; - /// Trait describing how values participate in Python's cyclic garbage collector. /// /// # Safety @@ -454,6 +453,35 @@ unsafe impl PyGcTraversable for Box { } } +// SAFETY: `Cow<'a, T>` either borrows a `T` or owns `T::Owned`; delegating to +// the active variant preserves traversal soundness. +unsafe impl<'a, T: ?Sized + PyGcTraversable + ToOwned> PyGcTraversable for Cow<'a, T> +where + T::Owned: PyGcTraversable, +{ + const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES || T::Owned::MAY_CONTAIN_CYCLES; + + fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + if Self::MAY_CONTAIN_CYCLES { + match self { + Self::Borrowed(value) if T::MAY_CONTAIN_CYCLES => value.traverse(visit)?, + Self::Owned(value) if T::Owned::MAY_CONTAIN_CYCLES => value.traverse(visit)?, + _ => {} + } + } + Ok(()) + } + + fn clear(&mut self) { + if T::Owned::MAY_CONTAIN_CYCLES { + match self { + Self::Borrowed(_) => {} + Self::Owned(value) => value.clear(), + } + } + } +} + // SAFETY: `Py` is a strong reference to a Python object and must always be visited. unsafe impl PyGcTraversable for Py { const MAY_CONTAIN_CYCLES: bool = true; diff --git a/tests/test_pygcintegration.rs b/tests/test_pygcintegration.rs index e96c22301fb..b1b75afda8a 100644 --- a/tests/test_pygcintegration.rs +++ b/tests/test_pygcintegration.rs @@ -3,6 +3,7 @@ use pyo3::prelude::*; use pyo3::types::PyDict; use pyo3::{PyTraverseError, PyVisit}; +use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet, LinkedList, VecDeque}; use std::marker::PhantomData; use std::sync::OnceLock; @@ -11,7 +12,7 @@ struct NotTraversable { value: i32, } -#[derive(PyGcTraversable, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Clone, PyGcTraversable, PartialEq, Eq, PartialOrd, Ord)] struct Leaf { value: i32, } @@ -43,6 +44,8 @@ struct Wrappers { marker: PhantomData, result: Result, array: [Leaf; 2], + cow_str: Cow<'static, str>, + cow_slice: Cow<'static, [Leaf]>, hash_map: HashMap, hash_set: HashSet, vec_deque: VecDeque, @@ -152,6 +155,8 @@ fn wrappers_compile_and_clear() { marker: PhantomData, result: Ok(Leaf { value: 7 }), array: [Leaf { value: 8 }, Leaf { value: 9 }], + cow_str: Cow::Borrowed("borrowed"), + cow_slice: Cow::Owned(vec![Leaf { value: 15 }]), hash_map, hash_set, vec_deque, @@ -162,6 +167,8 @@ fn wrappers_compile_and_clear() { assert_eq!(wrappers.map.len(), 1); assert_eq!(wrappers.set.len(), 1); assert!(wrappers.lock.get().is_some()); + assert_eq!(wrappers.cow_str, Cow::Borrowed("borrowed")); + assert_eq!(wrappers.cow_slice.len(), 1); assert_eq!(wrappers.hash_map.len(), 1); assert_eq!(wrappers.hash_set.len(), 1); assert_eq!(wrappers.vec_deque.len(), 1); From d117edca7cb012d5978c546e144ae00432912618 Mon Sep 17 00:00:00 2001 From: Bas Schoenmaeckers Date: Thu, 20 Aug 2026 12:51:53 +0200 Subject: [PATCH 6/7] Impl for Atomic types --- src/pyclass/gc.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/pyclass/gc.rs b/src/pyclass/gc.rs index 0a54a6531ba..805cfb0d01c 100644 --- a/src/pyclass/gc.rs +++ b/src/pyclass/gc.rs @@ -14,6 +14,10 @@ use core::{ marker::PhantomData, num::NonZero, ops::{Deref, DerefMut}, + sync::atomic::{ + AtomicBool, AtomicI16, AtomicI32, AtomicI64, AtomicI8, AtomicIsize, AtomicU16, AtomicU32, + AtomicU64, AtomicU8, AtomicUsize, + }, }; use std::{ collections::{HashMap, HashSet}, @@ -84,6 +88,17 @@ impl_py_gc_no_cycles!( PathBuf, OsStr, OsString, + AtomicBool, + AtomicI8, + AtomicU8, + AtomicI16, + AtomicU16, + AtomicI32, + AtomicU32, + AtomicI64, + AtomicU64, + AtomicIsize, + AtomicUsize, ); // SAFETY: Shared references do not own data; forwarding traversal is correct and From b99d64176b19054bfefbaea01e6b5825ff965cde Mon Sep 17 00:00:00 2001 From: Bas Schoenmaeckers Date: Sat, 22 Aug 2026 17:53:00 +0100 Subject: [PATCH 7/7] Remove `&T` impl --- src/pyclass/gc.rs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/pyclass/gc.rs b/src/pyclass/gc.rs index 805cfb0d01c..6c224bf5999 100644 --- a/src/pyclass/gc.rs +++ b/src/pyclass/gc.rs @@ -101,24 +101,6 @@ impl_py_gc_no_cycles!( AtomicUsize, ); -// SAFETY: Shared references do not own data; forwarding traversal is correct and -// clear is a no-op because `&T` cannot clear through immutable access. -unsafe impl PyGcTraversable for &T { - const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES; - - #[inline] - fn traverse(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { - if T::MAY_CONTAIN_CYCLES { - (**self).traverse(visit) - } else { - Ok(()) - } - } - - #[inline] - fn clear(&mut self) {} -} - // SAFETY: Mutable references can forward both traversal and clearing to `T`. unsafe impl PyGcTraversable for &mut T { const MAY_CONTAIN_CYCLES: bool = T::MAY_CONTAIN_CYCLES;