speicialize self conversion for descriptor slots for which CPython does ensure a bad type cannot be passed from Python

Author: MatthieuDartiailh

pyo3-macros-backend/src/method.rs

@@ -263,6 +263,7 @@ impl FnType {
         &self,
         cls: Option<&syn::Type>,
         error_mode: ExtractErrorMode,
+        descriptor_slot_receiver: bool,
         holders: &mut Holders,
         ctx: &Ctx,
     ) -> Option<TokenStream> {
@@ -272,6 +273,7 @@ impl FnType {
                 Some(st.receiver(
                     cls.expect("no class given for Fn with a \"self\" receiver"),
                     error_mode,
+                    descriptor_slot_receiver,
                     holders,
                     ctx,
                 ))
@@ -346,6 +348,7 @@ impl SelfType {
         &self,
         cls: &syn::Type,
         error_mode: ExtractErrorMode,
+        descriptor_slot_receiver: bool,
         holders: &mut Holders,
         ctx: &Ctx,
     ) -> TokenStream {
@@ -391,10 +394,27 @@ impl SelfType {
                     quote! { unsafe { #pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(#py, &#slf) } }
                 };
                 let pyo3_path = pyo3_path.to_tokens_spanned(*span);
-                error_mode.handle_error(
+                let receiver = if descriptor_slot_receiver {
+                    quote_spanned! { *span =>
+                        // Safety: descriptor slot wrappers are only installed on the descriptor
+                        // type itself. CPython calls those slots with `self` set to the
+                        // descriptor object found during lookup, and explicit Python calls to
+                        // `__get__`, `__set__`, and `__delete__` first pass through CPython's
+                        // slot wrapper, which rejects receivers of the wrong type before
+                        // reaching this generated wrapper.
+                        ::std::result::Result::<_, #pyo3_path::PyErr>::Ok(unsafe {
+                            #bound_ref.cast_unchecked::<#cls>()
+                        })
+                    }
+                } else {
                     quote_spanned! { *span =>
                         #bound_ref.cast::<#cls>()
                             .map_err(::std::convert::Into::<#pyo3_path::PyErr>::into)
+                    }
+                };
+                error_mode.handle_error(
+                    quote_spanned! { *span =>
+                        #receiver
                             .and_then(
                                 #[allow(clippy::unnecessary_fallible_conversions, reason = "anything implementing `TryFrom<BoundRef>` is permitted")]
                                 |bound| ::std::convert::TryFrom::try_from(bound).map_err(::std::convert::Into::into)
@@ -697,7 +717,7 @@ impl<'a> FnSpec<'a> {
         let rust_call = |args: Vec<TokenStream>, mut holders: Holders| {
             let self_arg = self
                 .tp
-                .self_arg(cls, ExtractErrorMode::Raise, &mut holders, ctx);
+                .self_arg(cls, ExtractErrorMode::Raise, false, &mut holders, ctx);
             let init_holders = holders.init_holders(ctx);
 
             // We must assign the output_span to the return value of the call,

pyo3-macros-backend/src/pymethod.rs

@@ -473,7 +473,7 @@ fn impl_clear_slot(cls: &syn::Type, spec: &FnSpec<'_>, ctx: &Ctx) -> syn::Result
         _ => bail_spanned!(spec.name.span() => "expected instance method for `__clear__` function"),
     };
     let mut holders = Holders::new();
-    let slf = self_type.receiver(cls, ExtractErrorMode::Raise, &mut holders, ctx);
+    let slf = self_type.receiver(cls, ExtractErrorMode::Raise, false, &mut holders, ctx);
 
     if let [arg, ..] = args {
         bail_spanned!(arg.ty().span() => "`__clear__` function expected to have no arguments");
@@ -571,7 +571,7 @@ fn impl_call_setter(
     ctx: &Ctx,
 ) -> syn::Result<TokenStream> {
     let (py_arg, args) = split_off_python_arg(&spec.signature.arguments);
-    let slf = self_type.receiver(cls, ExtractErrorMode::Raise, holders, ctx);
+    let slf = self_type.receiver(cls, ExtractErrorMode::Raise, false, holders, ctx);
 
     if args.is_empty() {
         bail_spanned!(spec.name.span() => "setter function expected to have one argument");
@@ -611,7 +611,7 @@ pub fn impl_py_setter_def(
                 span: Span::call_site(),
                 non_null: true,
             }
-            .receiver(cls, ExtractErrorMode::Raise, &mut holders, ctx);
+            .receiver(cls, ExtractErrorMode::Raise, false, &mut holders, ctx);
             if let Some(ident) = &field.ident {
                 // named struct field
                 quote!({ #slf.#ident = _val; })
@@ -757,7 +757,7 @@ fn impl_call_getter(
     ctx: &Ctx,
 ) -> syn::Result<TokenStream> {
     let (py_arg, args) = split_off_python_arg(&spec.signature.arguments);
-    let slf = self_type.receiver(cls, ExtractErrorMode::Raise, holders, ctx);
+    let slf = self_type.receiver(cls, ExtractErrorMode::Raise, false, holders, ctx);
     ensure_spanned!(
         args.is_empty(),
         args[0].ty().span() => "getter function can only have one argument (of type pyo3::Python)"
@@ -932,7 +932,7 @@ fn impl_call_deleter(
     ctx: &Ctx,
 ) -> Result<TokenStream> {
     let (py_arg, args) = split_off_python_arg(&spec.signature.arguments);
-    let slf = self_type.receiver(cls, ExtractErrorMode::Raise, holders, ctx);
+    let slf = self_type.receiver(cls, ExtractErrorMode::Raise, false, holders, ctx);
 
     if !args.is_empty() {
         bail_spanned!(spec.name.span() =>
@@ -1395,6 +1395,10 @@ impl SlotDef {
             spec,
             calling_convention,
             *extract_error_mode,
+            matches!(
+                calling_convention,
+                SlotCallingConvention::FixedArguments(&[Ty::MaybeNullObject, Ty::MaybeNullObject])
+            ),
             &mut holders,
             return_mode.as_ref(),
             ctx,
@@ -1425,11 +1429,16 @@ impl SlotDef {
     }
 }
 
+#[allow(
+    clippy::too_many_arguments,
+    reason = "slot wrapper generation needs the descriptor fast-path flag"
+)]
 fn generate_method_body(
     cls: &syn::Type,
     spec: &FnSpec<'_>,
     calling_convention: &SlotCallingConvention,
     extract_error_mode: ExtractErrorMode,
+    descriptor_slot_receiver: bool,
     holders: &mut Holders,
     // NB ignored if calling_convention is SlotCallingConvention::TpNew, possibly should merge into that enum
     return_mode: Option<&ReturnMode>,
@@ -1439,9 +1448,13 @@ fn generate_method_body(
         pyo3_path,
         output_span,
     } = ctx;
-    let self_arg = spec
-        .tp
-        .self_arg(Some(cls), extract_error_mode, holders, ctx);
+    let self_arg = spec.tp.self_arg(
+        Some(cls),
+        extract_error_mode,
+        descriptor_slot_receiver,
+        holders,
+        ctx,
+    );
     let rust_name = spec.name;
     let warnings = spec.warnings.build_py_warning(ctx);
 
@@ -1621,6 +1634,7 @@ impl SlotFragmentDef {
             spec,
             &SlotCallingConvention::FixedArguments(arguments),
             *extract_error_mode,
+            matches!(*fragment, "__set__" | "__delete__"),
             &mut holders,
             None,
             ctx,

tests/test_proto_methods.rs

@@ -813,6 +813,18 @@ assert c.counter.count == 4
 # __delete__
 del c.counter
 assert c.counter.count == 1
+
+# wrong receiver type should be rejected by CPython slot wrapper
+for call in (
+    lambda: Counter.__get__(object(), Class()),
+    lambda: Counter.__set__(object(), Class(), Counter()),
+    lambda: Counter.__delete__(object(), Class()),
+):
+    try:
+        call()
+        assert False, "expected TypeError"
+    except TypeError:
+        pass
 "#
         );
         let globals = PyModule::import(py, "__main__").unwrap().dict();