Fix member_descriptor to match CPython behavior (#6915)

descriptor.rs:
- Add __objclass__, __name__ (pymember) and __reduce__ (pymethod)
- Add type check (descr_check) in descr_get; simplify None branch
- Remove incorrect BASETYPE flag
- Add MemberKind::Object (_Py_T_OBJECT = 6)
- Prevent Bool slot deletion (TypeError)
- Raise AttributeError on ObjectEx deletion when already None

pyclass.rs (derive-impl):
- Remove duplicate MemberKind enum; use MemberKindStr (Option<String>)
- Simplify MemberNursery map key from (String, MemberKind) to String
- Support #[pymember(type="object")] for _Py_T_OBJECT semantics

test_inspect.py:
- Remove expectedFailure from test_getdoc (now passing)

Lib/test/test_inspect/test_inspect.py

@@ -672,7 +672,6 @@ class TestRetrievingSourceCode(GetSourceBase):
                                      ('lobbest', mod.lobbest),
                                      ('spam', mod.spam)])
 
-    @unittest.expectedFailure  # TODO: RUSTPYTHON
     @unittest.skipIf(sys.flags.optimize >= 2,
                      "Docstrings are omitted with -O2 and above")
     def test_getdoc(self):

crates/derive-impl/src/pyclass.rs

@@ -1207,13 +1207,18 @@ where
         let item_meta = MemberItemMeta::from_attr(ident.clone(), &item_attr)?;
 
         let (py_name, member_item_kind) = item_meta.member_name()?;
-        let member_kind = match item_meta.member_kind()? {
-            Some(s) => match s.as_str() {
-                "bool" => MemberKind::Bool,
-                _ => unreachable!(),
-            },
-            _ => MemberKind::ObjectEx,
-        };
+        let member_kind = item_meta.member_kind()?;
+        if let Some(ref s) = member_kind {
+            match s.as_str() {
+                "bool" | "object" => {}
+                other => {
+                    return Err(self.new_syn_error(
+                        args.item.span(),
+                        &format!("unknown member type '{other}'"),
+                    ));
+                }
+            }
+        }
 
         // Add #[allow(non_snake_case)] for setter methods
         if matches!(member_item_kind, MemberItemKind::Set) {
@@ -1393,37 +1398,47 @@ impl ToTokens for GetSetNursery {
     }
 }
 
+/// Member kind as string, matching `rustpython_vm::builtins::descriptor::MemberKind` variants.
+/// None means ObjectEx (default). Valid values: "bool", "object".
+type MemberKindStr = Option<String>;
+
 #[derive(Default)]
-#[allow(clippy::type_complexity)]
 struct MemberNursery {
-    map: HashMap<(String, MemberKind), (Option<Ident>, Option<Ident>)>,
+    map: HashMap<String, MemberNurseryEntry>,
     validated: bool,
 }
 
+struct MemberNurseryEntry {
+    kind: MemberKindStr,
+    getter: Option<Ident>,
+    setter: Option<Ident>,
+}
+
 enum MemberItemKind {
     Get,
     Set,
 }
 
-#[derive(Eq, PartialEq, Hash)]
-enum MemberKind {
-    Bool,
-    ObjectEx,
-}
-
 impl MemberNursery {
     fn add_item(
         &mut self,
         name: String,
         kind: MemberItemKind,
-        member_kind: MemberKind,
+        member_kind: MemberKindStr,
         item_ident: Ident,
     ) -> Result<()> {
         assert!(!self.validated, "new item is not allowed after validation");
-        let entry = self.map.entry((name.clone(), member_kind)).or_default();
+        let entry = self
+            .map
+            .entry(name.clone())
+            .or_insert_with(|| MemberNurseryEntry {
+                kind: member_kind,
+                getter: None,
+                setter: None,
+            });
         let func = match kind {
-            MemberItemKind::Get => &mut entry.0,
-            MemberItemKind::Set => &mut entry.1,
+            MemberItemKind::Get => &mut entry.getter,
+            MemberItemKind::Set => &mut entry.setter,
         };
         if func.is_some() {
             bail_span!(item_ident, "Multiple member accessors with name '{}'", name);
@@ -1434,10 +1449,10 @@ impl MemberNursery {
 
     fn validate(&mut self) -> Result<()> {
         let mut errors = Vec::new();
-        for ((name, _), (getter, setter)) in &self.map {
-            if getter.is_none() {
+        for (name, entry) in &self.map {
+            if entry.getter.is_none() {
                 errors.push(err_span!(
-                    setter.as_ref().unwrap(),
+                    entry.setter.as_ref().unwrap(),
                     "Member '{}' is missing a getter",
                     name
                 ));
@@ -1452,30 +1467,31 @@ impl MemberNursery {
 impl ToTokens for MemberNursery {
     fn to_tokens(&self, tokens: &mut TokenStream) {
         assert!(self.validated, "Call `validate()` before token generation");
-        let properties = self
-            .map
-            .iter()
-            .map(|((name, member_kind), (getter, setter))| {
-                let setter = match setter {
-                    Some(setter) => quote_spanned! { setter.span() => Some(Self::#setter)},
-                    None => quote! { None },
-                };
-                let member_kind = match member_kind {
-                    MemberKind::Bool => {
-                        quote!(::rustpython_vm::builtins::descriptor::MemberKind::Bool)
-                    }
-                    MemberKind::ObjectEx => {
-                        quote!(::rustpython_vm::builtins::descriptor::MemberKind::ObjectEx)
-                    }
-                };
-                quote_spanned! { getter.span() =>
-                    class.set_str_attr(
-                        #name,
-                        ctx.new_member(#name, #member_kind, Self::#getter, #setter, class),
-                        ctx,
-                    );
+        let properties = self.map.iter().map(|(name, entry)| {
+            let setter = match &entry.setter {
+                Some(setter) => quote_spanned! { setter.span() => Some(Self::#setter)},
+                None => quote! { None },
+            };
+            let member_kind = match entry.kind.as_deref() {
+                Some("bool") => {
+                    quote!(::rustpython_vm::builtins::descriptor::MemberKind::Bool)
                 }
-            });
+                Some("object") => {
+                    quote!(::rustpython_vm::builtins::descriptor::MemberKind::Object)
+                }
+                _ => {
+                    quote!(::rustpython_vm::builtins::descriptor::MemberKind::ObjectEx)
+                }
+            };
+            let getter = entry.getter.as_ref().unwrap();
+            quote_spanned! { getter.span() =>
+                class.set_str_attr(
+                    #name,
+                    ctx.new_member(#name, #member_kind, Self::#getter, #setter, class),
+                    ctx,
+                );
+            }
+        });
         tokens.extend(properties);
     }
 }

crates/vm/src/builtins/descriptor.rs

@@ -170,6 +170,7 @@ impl Representable for PyMethodDescriptor {
 
 #[derive(Debug)]
 pub enum MemberKind {
+    Object = 6,
     Bool = 14,
     ObjectEx = 16,
 }
@@ -253,11 +254,20 @@ fn calculate_qualname(descr: &PyDescriptorOwned, vm: &VirtualMachine) -> PyResul
     }
 }
 
-#[pyclass(
-    with(GetDescriptor, Representable),
-    flags(BASETYPE, DISALLOW_INSTANTIATION)
-)]
+#[pyclass(with(GetDescriptor, Representable), flags(DISALLOW_INSTANTIATION))]
 impl PyMemberDescriptor {
+    #[pymember]
+    fn __objclass__(vm: &VirtualMachine, zelf: PyObjectRef) -> PyResult {
+        let zelf: &Py<Self> = zelf.try_to_value(vm)?;
+        Ok(zelf.common.typ.clone().into())
+    }
+
+    #[pymember]
+    fn __name__(vm: &VirtualMachine, zelf: PyObjectRef) -> PyResult {
+        let zelf: &Py<Self> = zelf.try_to_value(vm)?;
+        Ok(zelf.common.name.to_owned().into())
+    }
+
     #[pygetset]
     fn __doc__(&self) -> Option<String> {
         self.member.doc.to_owned()
@@ -276,6 +286,23 @@ impl PyMemberDescriptor {
         })
     }
 
+    #[pymethod]
+    fn __reduce__(&self, vm: &VirtualMachine) -> PyResult {
+        let builtins_getattr = vm.builtins.get_attr("getattr", vm)?;
+        Ok(vm
+            .ctx
+            .new_tuple(vec![
+                builtins_getattr,
+                vm.ctx
+                    .new_tuple(vec![
+                        self.common.typ.clone().into(),
+                        vm.ctx.new_str(self.common.name.as_str()).into(),
+                    ])
+                    .into(),
+            ])
+            .into())
+    }
+
     #[pyslot]
     fn descr_set(
         zelf: &PyObject,
@@ -306,6 +333,7 @@ fn get_slot_from_object(
     vm: &VirtualMachine,
 ) -> PyResult {
     let slot = match member.kind {
+        MemberKind::Object => obj.get_slot(offset).unwrap_or_else(|| vm.ctx.none()),
         MemberKind::Bool => obj
             .get_slot(offset)
             .unwrap_or_else(|| vm.ctx.new_bool(false).into()),
@@ -325,25 +353,38 @@ fn set_slot_at_object(
     vm: &VirtualMachine,
 ) -> PyResult<()> {
     match member.kind {
+        MemberKind::Object => match value {
+            PySetterValue::Assign(v) => {
+                obj.set_slot(offset, Some(v));
+            }
+            PySetterValue::Delete => {
+                obj.set_slot(offset, None);
+            }
+        },
         MemberKind::Bool => {
             match value {
                 PySetterValue::Assign(v) => {
                     if !v.class().is(vm.ctx.types.bool_type) {
                         return Err(vm.new_type_error("attribute value type must be bool"));
                     }
-
                     obj.set_slot(offset, Some(v))
                 }
-                PySetterValue::Delete => obj.set_slot(offset, None),
+                PySetterValue::Delete => {
+                    return Err(vm.new_type_error("can't delete numeric/char attribute".to_owned()));
+                }
             };
         }
-        MemberKind::ObjectEx => {
-            let value = match value {
-                PySetterValue::Assign(v) => Some(v),
-                PySetterValue::Delete => None,
-            };
-            obj.set_slot(offset, value);
-        }
+        MemberKind::ObjectEx => match value {
+            PySetterValue::Assign(v) => {
+                obj.set_slot(offset, Some(v));
+            }
+            PySetterValue::Delete => {
+                if obj.get_slot(offset).is_none() {
+                    return Err(vm.new_attribute_error(member.name.clone()));
+                }
+                obj.set_slot(offset, None);
+            }
+        },
     }
 
     Ok(())
@@ -364,26 +405,23 @@ impl GetDescriptor for PyMemberDescriptor {
     fn descr_get(
         zelf: PyObjectRef,
         obj: Option<PyObjectRef>,
-        cls: Option<PyObjectRef>,
+        _cls: Option<PyObjectRef>,
         vm: &VirtualMachine,
     ) -> PyResult {
         let descr = Self::_as_pyref(&zelf, vm)?;
         match obj {
-            Some(x) => descr.member.get(x, vm),
-            None => {
-                // When accessed from class (not instance), for __doc__ member descriptor,
-                // return the class's docstring if available
-                // When accessed from class (not instance), check if the class has
-                // an attribute with the same name as this member descriptor
-                if let Some(cls) = cls
-                    && let Ok(cls_type) = cls.downcast::<PyType>()
-                    && let Some(interned) = vm.ctx.interned_str(descr.member.name.as_str())
-                    && let Some(attr) = cls_type.attributes.read().get(&interned)
-                {
-                    return Ok(attr.clone());
+            Some(x) => {
+                if !x.class().fast_issubclass(&descr.common.typ) {
+                    return Err(vm.new_type_error(format!(
+                        "descriptor '{}' for '{}' objects doesn't apply to a '{}' object",
+                        descr.common.name,
+                        descr.common.typ.name(),
+                        x.class().name()
+                    )));
                 }
-                Ok(zelf)
+                descr.member.get(x, vm)
             }
+            None => Ok(zelf),
         }
     }
 }