Merge pull request #2159 from davidhewitt/gc-protocol

pymethods: support gc protocol
PyO3 · Feb 15, 2022 · 9704c86 · 9704c86
2 parents 8218b5a + 79123b3
commit 9704c86
Show file tree

Hide file tree

Showing 16 changed files with 228 additions and 181 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `wrap_pyfunction!` can now wrap a `#[pyfunction]` which is implemented in a different Rust module or crate. [#2091](https://github.com/PyO3/pyo3/pull/2091)
 - Add `PyAny::contains` method (`in` operator for `PyAny`). [#2115](https://github.com/PyO3/pyo3/pull/2115)
 - Add `PyMapping::contains` method (`in` operator for `PyMapping`). [#2133](https://github.com/PyO3/pyo3/pull/2133)
+- Add garbage collection magic methods `__traverse__` and `__clear__` to `#[pymethods]`. [#2159](https://github.com/PyO3/pyo3/pull/2159)
 
 ### Changed
 
@@ -69,6 +70,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - `PyTZInfo_CheckExact`
   - `PyDateTime_FromTimestamp`
   - `PyDate_FromTimestamp`
+- Deprecate the `gc` option for `pyclass` (e.g. `#[pyclass(gc)]`). Just implement a `__traverse__` `#[pymethod]`. [#2159](https://github.com/PyO3/pyo3/pull/2159)
 - The `ml_meth` field of `PyMethodDef` is now represented by the `PyMethodDefPointer` union [2166](https://github.com/PyO3/pyo3/pull/2166)
 
 ### Removed

diff --git a/guide/src/class.md b/guide/src/class.md
@@ -967,7 +967,6 @@ impl pyo3::IntoPy<PyObject> for MyClass {
 
 impl pyo3::impl_::pyclass::PyClassImpl for MyClass {
     const DOC: &'static str = "Class for demonstration\u{0}";
-    const IS_GC: bool = false;
     const IS_BASETYPE: bool = false;
     const IS_SUBCLASS: bool = false;
     type Layout = PyCell<MyClass>;

diff --git a/guide/src/class/protocols.md b/guide/src/class/protocols.md
@@ -175,7 +175,8 @@ given signatures should be interpreted as follows:
 
 #### Garbage Collector Integration
 
-TODO; see [#1884](https://github.com/PyO3/pyo3/issues/1884)
+  - `__traverse__(<self>, visit: pyo3::class::gc::PyVisit) -> Result<(), pyo3::class::gc::PyTraverseError>`
+  - `__clear__(<self>) -> ()`
 
 ### `#[pyproto]` traits
 
@@ -444,19 +445,6 @@ impl PyGCProtocol for ClassWithGCSupport {
 }
 ```
 
-Special protocol trait implementations have to be annotated with the `#[pyproto]` attribute.
-
-It is also possible to enable GC for custom classes using the `gc` parameter of the `pyclass` attribute.
-i.e. `#[pyclass(gc)]`. In that case instances of custom class participate in Python garbage
-collection, and it is possible to track them with `gc` module methods. When using the `gc` parameter,
-it is *required* to implement the `PyGCProtocol` trait, failure to do so will result in an error
-at compile time:
-
-```compile_fail
-#[pyclass(gc)]
-struct GCTracked {} // Fails because it does not implement PyGCProtocol
-```
-
 #### Iterator Types
 
 Iterators can be defined using the

diff --git a/pyo3-macros-backend/src/deprecations.rs b/pyo3-macros-backend/src/deprecations.rs
@@ -3,12 +3,14 @@ use quote::{quote_spanned, ToTokens};
 
 pub enum Deprecation {
     CallAttribute,
+    PyClassGcOption,
 }
 
 impl Deprecation {
     fn ident(&self, span: Span) -> syn::Ident {
         let string = match self {
             Deprecation::CallAttribute => "CALL_ATTRIBUTE",
+            Deprecation::PyClassGcOption => "PYCLASS_GC_OPTION",
         };
         syn::Ident::new(string, span)
     }

diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs
@@ -3,7 +3,7 @@
 use crate::attributes::{
     self, take_pyo3_options, CrateAttribute, NameAttribute, TextSignatureAttribute,
 };
-use crate::deprecations::Deprecations;
+use crate::deprecations::{Deprecation, Deprecations};
 use crate::konst::{ConstAttributes, ConstSpec};
 use crate::pyimpl::{gen_default_items, gen_py_const, PyClassMethodsType};
 use crate::pymethod::{impl_py_getter_def, impl_py_setter_def, PropertyType};
@@ -29,12 +29,12 @@ pub struct PyClassArgs {
     pub base: syn::TypePath,
     pub has_dict: bool,
     pub has_weaklist: bool,
-    pub is_gc: bool,
     pub is_basetype: bool,
     pub has_extends: bool,
     pub has_unsendable: bool,
     pub module: Option<syn::LitStr>,
     pub class_kind: PyClassKind,
+    pub deprecations: Deprecations,
 }
 
 impl PyClassArgs {
@@ -63,11 +63,11 @@ impl PyClassArgs {
             base: parse_quote! { _pyo3::PyAny },
             has_dict: false,
             has_weaklist: false,
-            is_gc: false,
             is_basetype: false,
             has_extends: false,
             has_unsendable: false,
             class_kind,
+            deprecations: Deprecations::new(),
         }
     }
 
@@ -158,9 +158,9 @@ impl PyClassArgs {
     fn add_path(&mut self, exp: &syn::ExprPath) -> syn::Result<()> {
         let flag = exp.path.segments.first().unwrap().ident.to_string();
         match flag.as_str() {
-            "gc" => {
-                self.is_gc = true;
-            }
+            "gc" => self
+                .deprecations
+                .push(Deprecation::PyClassGcOption, exp.span()),
             "weakref" => {
                 self.has_weaklist = true;
             }
@@ -757,7 +757,6 @@ impl<'a> PyClassImplsBuilder<'a> {
             self.impl_into_py(),
             self.impl_pyclassimpl(),
             self.impl_freelist(),
-            self.impl_gc(),
         ]
         .into_iter()
         .collect()
@@ -826,7 +825,6 @@ impl<'a> PyClassImplsBuilder<'a> {
     fn impl_pyclassimpl(&self) -> TokenStream {
         let cls = self.cls;
         let doc = self.doc.as_ref().map_or(quote! {"\0"}, |doc| quote! {#doc});
-        let is_gc = self.attr.is_gc;
         let is_basetype = self.attr.is_basetype;
         let base = &self.attr.base;
         let is_subclass = self.attr.has_extends;
@@ -904,10 +902,11 @@ impl<'a> PyClassImplsBuilder<'a> {
         let default_slots = &self.default_slots;
         let freelist_slots = self.freelist_slots();
 
+        let deprecations = &self.attr.deprecations;
+
         quote! {
             impl _pyo3::impl_::pyclass::PyClassImpl for #cls {
                 const DOC: &'static str = #doc;
-                const IS_GC: bool = #is_gc;
                 const IS_BASETYPE: bool = #is_basetype;
                 const IS_SUBCLASS: bool = #is_subclass;
 
@@ -919,6 +918,7 @@ impl<'a> PyClassImplsBuilder<'a> {
                 fn for_all_items(visitor: &mut dyn ::std::ops::FnMut(& _pyo3::impl_::pyclass::PyClassItems)) {
                     use _pyo3::impl_::pyclass::*;
                     let collector = PyClassImplCollector::<Self>::new();
+                    #deprecations;
                     static INTRINSIC_ITEMS: PyClassItems = PyClassItems {
                         methods: &[#(#default_methods),*],
                         slots: &[#(#default_slots),* #(#freelist_slots),*],
@@ -981,26 +981,6 @@ impl<'a> PyClassImplsBuilder<'a> {
             Vec::new()
         }
     }
-
-    /// Enforce at compile time that PyGCProtocol is implemented
-    fn impl_gc(&self) -> TokenStream {
-        let cls = self.cls;
-        let attr = self.attr;
-        if attr.is_gc {
-            let closure_name = format!("__assertion_closure_{}", cls);
-            let closure_token = syn::Ident::new(&closure_name, Span::call_site());
-            quote! {
-                fn #closure_token() {
-                    use _pyo3::class;
-
-                    fn _assert_implements_protocol<'p, T: _pyo3::class::PyGCProtocol<'p>>() {}
-                    _assert_implements_protocol::<#cls>();
-                }
-            }
-        } else {
-            quote! {}
-        }
-    }
 }
 
 fn define_inventory_class(inventory_class_name: &syn::Ident) -> TokenStream {