refactor: include __new__ as slot

PyO3 · Feb 9, 2022 · f5b2a88 · f5b2a88
1 parent 75e4458
commit f5b2a88
Show file tree

Hide file tree

Showing 8 changed files with 55 additions and 63 deletions.
diff --git a/guide/src/class.md b/guide/src/class.md
@@ -981,11 +981,6 @@ impl pyo3::impl_::pyclass::PyClassImpl for MyClass {
         visitor(&INTRINSIC_ITEMS);
         visitor(collector.py_methods());
     }
-    fn get_new() -> Option<pyo3::ffi::newfunc> {
-        use pyo3::impl_::pyclass::*;
-        let collector = PyClassImplCollector::<Self>::new();
-        collector.new_impl()
-    }
 }
 # Python::with_gil(|py| {
 #     let cls = py.get_type::<MyClass>();

diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs
@@ -927,11 +927,6 @@ impl<'a> PyClassImplsBuilder<'a> {
                     #pymethods_items
                     #pyproto_items
                 }
-                fn get_new() -> ::std::option::Option<_pyo3::ffi::newfunc> {
-                    use _pyo3::impl_::pyclass::*;
-                    let collector = PyClassImplCollector::<Self>::new();
-                    collector.new_impl()
-                }
 
                 #dict_offset
 

diff --git a/pyo3-macros-backend/src/pyimpl.rs b/pyo3-macros-backend/src/pyimpl.rs
@@ -108,10 +108,6 @@ pub fn impl_methods(
                         let attrs = get_cfg_attributes(&meth.attrs);
                         methods.push(quote!(#(#attrs)* #token_stream));
                     }
-                    GeneratedPyMethod::TraitImpl(token_stream) => {
-                        let attrs = get_cfg_attributes(&meth.attrs);
-                        trait_impls.push(quote!(#(#attrs)* #token_stream));
-                    }
                     GeneratedPyMethod::SlotTraitImpl(method_name, token_stream) => {
                         implemented_proto_fragments.insert(method_name);
                         let attrs = get_cfg_attributes(&meth.attrs);
@@ -204,7 +200,7 @@ pub fn gen_default_items<'a>(
             GeneratedPyMethod::SlotTraitImpl(..) => {
                 panic!("SlotFragment methods cannot have default implementation!")
             }
-            GeneratedPyMethod::Method(_) | GeneratedPyMethod::TraitImpl(_) => {
+            GeneratedPyMethod::Method(_) => {
                 panic!("Only protocol methods can have default implementation!")
             }
         }

diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs
@@ -20,7 +20,6 @@ use syn::{ext::IdentExt, spanned::Spanned, Result};
 pub enum GeneratedPyMethod {
     Method(TokenStream),
     Proto(TokenStream),
-    TraitImpl(TokenStream),
     SlotTraitImpl(String, TokenStream),
 }
 
@@ -128,7 +127,7 @@ pub fn gen_py_method(
             Some(quote!(_pyo3::ffi::METH_STATIC)),
         )?),
         // special prototypes
-        (_, FnType::FnNew) => GeneratedPyMethod::TraitImpl(impl_py_method_def_new(cls, spec)?),
+        (_, FnType::FnNew) => GeneratedPyMethod::Proto(impl_py_method_def_new(cls, spec)?),
 
         (_, FnType::Getter(self_type)) => GeneratedPyMethod::Method(impl_py_getter_def(
             cls,
@@ -191,18 +190,18 @@ pub fn impl_py_method_def(
 }
 
 fn impl_py_method_def_new(cls: &syn::Type, spec: &FnSpec) -> Result<TokenStream> {
-    let wrapper_ident = syn::Ident::new("__wrap", Span::call_site());
+    let wrapper_ident = syn::Ident::new("__pymethod__new__", Span::call_site());
     let wrapper = spec.get_wrapper_function(&wrapper_ident, Some(cls))?;
-    Ok(quote! {
-        impl _pyo3::impl_::pyclass::PyClassNewImpl<#cls> for _pyo3::impl_::pyclass::PyClassImplCollector<#cls> {
-            fn new_impl(self) -> ::std::option::Option<_pyo3::ffi::newfunc> {
-                ::std::option::Option::Some({
-                    #wrapper
-                    #wrapper_ident
-                })
-            }
+    Ok(quote! {{
+        impl #cls {
+            #[doc(hidden)]
+            #wrapper
         }
-    })
+        _pyo3::ffi::PyType_Slot {
+            slot: _pyo3::ffi::Py_tp_new,
+            pfunc: #cls::#wrapper_ident as _pyo3::ffi::newfunc as _
+        }
+    }})
 }
 
 fn impl_call_slot(cls: &syn::Type, mut spec: FnSpec) -> Result<TokenStream> {

diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs
@@ -178,10 +178,6 @@ pub trait PyClassImpl: Sized {
 
     fn for_all_items(visitor: &mut dyn FnMut(&PyClassItems));
 
-    #[inline]
-    fn get_new() -> Option<ffi::newfunc> {
-        None
-    }
     #[inline]
     fn dict_offset() -> Option<ffi::Py_ssize_t> {
         None
@@ -194,16 +190,6 @@ pub trait PyClassImpl: Sized {
 
 // Traits describing known special methods.
 
-pub trait PyClassNewImpl<T> {
-    fn new_impl(self) -> Option<ffi::newfunc>;
-}
-
-impl<T> PyClassNewImpl<T> for &'_ PyClassImplCollector<T> {
-    fn new_impl(self) -> Option<ffi::newfunc> {
-        None
-    }
-}
-
 macro_rules! slot_fragment_trait {
     ($trait_name:ident, $($default_method:tt)*) => {
         #[allow(non_camel_case_types)]
@@ -815,19 +801,6 @@ impl<T: PyClass> PyClassBaseType for T {
     type Initializer = crate::pyclass_init::PyClassInitializer<Self>;
 }
 
-/// Default new implementation
-pub(crate) unsafe extern "C" fn fallback_new(
-    _subtype: *mut ffi::PyTypeObject,
-    _args: *mut ffi::PyObject,
-    _kwds: *mut ffi::PyObject,
-) -> *mut ffi::PyObject {
-    crate::callback_body!(py, {
-        Err::<(), _>(crate::exceptions::PyTypeError::new_err(
-            "No constructor defined",
-        ))
-    })
-}
-
 /// Implementation of tp_dealloc for all pyclasses
 pub(crate) unsafe extern "C" fn tp_dealloc<T: PyClass>(obj: *mut ffi::PyObject) {
     crate::callback_body!(py, T::Layout::tp_dealloc(obj, py))

diff --git a/src/pyclass.rs b/src/pyclass.rs
@@ -3,8 +3,8 @@ use crate::{
     callback::IntoPyCallbackOutput,
     ffi,
     impl_::pyclass::{
-        assign_sequence_item_from_mapping, fallback_new, get_sequence_item_from_mapping,
-        tp_dealloc, PyClassDict, PyClassImpl, PyClassItems, PyClassWeakRef,
+        assign_sequence_item_from_mapping, get_sequence_item_from_mapping, tp_dealloc, PyClassDict,
+        PyClassImpl, PyClassItems, PyClassWeakRef,
     },
     IntoPy, IntoPyPointer, PyCell, PyErr, PyMethodDefType, PyNativeType, PyObject, PyResult,
     PyTypeInfo, Python,
@@ -49,7 +49,6 @@ where
             T::NAME,
             T::BaseType::type_object_raw(py),
             std::mem::size_of::<T::Layout>(),
-            T::get_new(),
             tp_dealloc::<T>,
             T::dict_offset(),
             T::weaklist_offset(),
@@ -71,7 +70,6 @@ unsafe fn create_type_object_impl(
     name: &str,
     base_type_object: *mut ffi::PyTypeObject,
     basicsize: usize,
-    tp_new: Option<ffi::newfunc>,
     tp_dealloc: ffi::destructor,
     dict_offset: Option<ffi::Py_ssize_t>,
     weaklist_offset: Option<ffi::Py_ssize_t>,
@@ -90,11 +88,6 @@ unsafe fn create_type_object_impl(
         push_slot(&mut slots, ffi::Py_tp_doc, doc as _);
     }
 
-    push_slot(
-        &mut slots,
-        ffi::Py_tp_new,
-        tp_new.unwrap_or(fallback_new) as _,
-    );
     push_slot(&mut slots, ffi::Py_tp_dealloc, tp_dealloc as _);
 
     #[cfg(Py_3_9)]
@@ -121,6 +114,7 @@ unsafe fn create_type_object_impl(
     }
 
     // protocol methods
+    let mut has_new = false;
     let mut has_getitem = false;
     let mut has_setitem = false;
     let mut has_gc_methods = false;
@@ -131,6 +125,7 @@ unsafe fn create_type_object_impl(
 
     for_all_items(&mut |items| {
         for slot in items.slots {
+            has_new |= slot.slot == ffi::Py_tp_new;
             has_getitem |= slot.slot == ffi::Py_mp_subscript;
             has_setitem |= slot.slot == ffi::Py_mp_ass_subscript;
             has_gc_methods |= slot.slot == ffi::Py_tp_clear || slot.slot == ffi::Py_tp_traverse;
@@ -171,6 +166,10 @@ unsafe fn create_type_object_impl(
         );
     }
 
+    if !has_new {
+        push_slot(&mut slots, ffi::Py_tp_new, no_constructor_defined as _);
+    }
+
     // Add empty sentinel at the end
     push_slot(&mut slots, 0, ptr::null_mut());
 
@@ -550,3 +549,16 @@ where
         }
     }
 }
+
+/// Default new implementation
+pub(crate) unsafe extern "C" fn no_constructor_defined(
+    _subtype: *mut ffi::PyTypeObject,
+    _args: *mut ffi::PyObject,
+    _kwds: *mut ffi::PyObject,
+) -> *mut ffi::PyObject {
+    crate::callback_body!(py, {
+        Err::<(), _>(crate::exceptions::PyTypeError::new_err(
+            "No constructor defined",
+        ))
+    })
+}
diff --git a/tests/ui/invalid_pymethods.rs b/tests/ui/invalid_pymethods.rs
@@ -119,4 +119,15 @@ impl MyClass {
     fn default_arg_before_required(&self, has_default: isize, required: isize) {}
 }
 
+struct TwoNew { }
+
+#[pymethods]
+impl TwoNew {
+    #[new]
+    fn new_1() -> Self { Self { } }
+
+    #[new]
+    fn new_2() -> Self { Self { } }
+}
+
 fn main() {}
diff --git a/tests/ui/invalid_pymethods.stderr b/tests/ui/invalid_pymethods.stderr
@@ -101,3 +101,14 @@ error: `pass_module` cannot be used on Python methods
     |
 112 |     #[pyo3(pass_module)]
     |            ^^^^^^^^^^^
+
+error[E0592]: duplicate definitions with name `__pymethod__new__`
+   --> tests/ui/invalid_pymethods.rs:124:1
+    |
+124 | #[pymethods]
+    | ^^^^^^^^^^^^
+    | |
+    | duplicate definitions for `__pymethod__new__`
+    | other definition for `__pymethod__new__`
+    |
+    = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)