From e629c2a1ba41489c2f580c3218d27604baf2b01d Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 31 Jan 2026 23:02:45 +0900 Subject: [PATCH] Fix member_descriptor to match CPython behavior 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) - 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 | 1 - crates/derive-impl/src/pyclass.rs | 106 +++++++++++++++----------- crates/vm/src/builtins/descriptor.rs | 92 +++++++++++++++------- 3 files changed, 126 insertions(+), 73 deletions(-) diff --git a/Lib/test/test_inspect/test_inspect.py b/Lib/test/test_inspect/test_inspect.py index 595966e3405..13ae2e3cb9c 100644 --- a/Lib/test/test_inspect/test_inspect.py +++ b/Lib/test/test_inspect/test_inspect.py @@ -672,7 +672,6 @@ def test_getfunctions(self): ('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): diff --git a/crates/derive-impl/src/pyclass.rs b/crates/derive-impl/src/pyclass.rs index 31a7fcc7569..f88fa059817 100644 --- a/crates/derive-impl/src/pyclass.rs +++ b/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; + #[derive(Default)] -#[allow(clippy::type_complexity)] struct MemberNursery { - map: HashMap<(String, MemberKind), (Option, Option)>, + map: HashMap, validated: bool, } +struct MemberNurseryEntry { + kind: MemberKindStr, + getter: Option, + setter: Option, +} + 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); } } diff --git a/crates/vm/src/builtins/descriptor.rs b/crates/vm/src/builtins/descriptor.rs index 89dafdd14b7..e1b92746a0f 100644 --- a/crates/vm/src/builtins/descriptor.rs +++ b/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 = zelf.try_to_value(vm)?; + Ok(zelf.common.typ.clone().into()) + } + + #[pymember] + fn __name__(vm: &VirtualMachine, zelf: PyObjectRef) -> PyResult { + let zelf: &Py = zelf.try_to_value(vm)?; + Ok(zelf.common.name.to_owned().into()) + } + #[pygetset] fn __doc__(&self) -> Option { 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), - }; - } - MemberKind::ObjectEx => { - let value = match value { - PySetterValue::Assign(v) => Some(v), - PySetterValue::Delete => None, + PySetterValue::Delete => { + return Err(vm.new_type_error("can't delete numeric/char attribute".to_owned())); + } }; - 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, - cls: Option, + _cls: Option, 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::() - && 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), } } }