From 0878b8e8460d081477b119fa357d2d42f95248b5 Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Thu, 30 Oct 2025 19:11:57 +0900 Subject: [PATCH 1/3] Implement property.__name__ attribute Add getter and setter for the __name__ attribute on property objects. The getter returns the explicitly set name if available, otherwise falls back to the getter function's __name__. Raises AttributeError if no name is available, matching CPython 3.13 behavior. The implementation handles edge cases: - Returns None when explicitly set to None - Propagates non-AttributeError exceptions from getter's __getattr__ - Raises property-specific AttributeError when getter lacks __name__ This fix enables test_property_name in test_property.py to pass. --- Lib/test/test_property.py | 1 - vm/src/builtins/property.rs | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py index 65153395b7..49dc2331e9 100644 --- a/Lib/test/test_property.py +++ b/Lib/test/test_property.py @@ -201,7 +201,6 @@ def test_gh_115618(self): self.assertIsNone(prop.fdel) self.assertAlmostEqual(gettotalrefcount() - refs_before, 0, delta=10) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_property_name(self): def getter(self): return 42 diff --git a/vm/src/builtins/property.rs b/vm/src/builtins/property.rs index 1568cb08d8..746c8f045b 100644 --- a/vm/src/builtins/property.rs +++ b/vm/src/builtins/property.rs @@ -143,6 +143,41 @@ impl PyProperty { self.deleter.read().clone() } + #[pygetset(name = "__name__")] + fn name_getter(&self, vm: &VirtualMachine) -> PyResult { + // If name was explicitly set via __set_name__ or direct assignment + // (even to None), return it as-is + if let Some(name) = self.name.read().as_ref() { + return Ok(name.clone()); + } + + // Fallback to getter's __name__ if available + if let Some(getter) = self.getter.read().as_ref() { + match getter.get_attr("__name__", vm) { + Ok(name) => Ok(name), + Err(e) => { + // If it's an AttributeError from the getter, convert to property's AttributeError + // Otherwise, propagate the original exception (e.g., RuntimeError) + if e.class().is(vm.ctx.exceptions.attribute_error) { + return Err(vm.new_attribute_error( + "'property' object has no attribute '__name__'".to_owned(), + )); + } + + Err(e) + } + } + } else { + // If no getter, raise AttributeError + Err(vm.new_attribute_error("'property' object has no attribute '__name__'".to_owned())) + } + } + + #[pygetset(name = "__name__", setter)] + fn name_setter(&self, value: PyObjectRef) { + *self.name.write() = Some(value); + } + fn doc_getter(&self) -> Option { self.doc.read().clone() } From 6bcb2b63fc5c3609cc21765ebeed2c5fd99ecaa1 Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Sat, 1 Nov 2025 00:51:16 +0900 Subject: [PATCH 2/3] Refactor to use get_property_name in __name__ implementation Consolidate duplicate logic by making name_getter() use the existing get_property_name() helper method. This eliminates code duplication and improves maintainability. Changes: - Update get_property_name() to return PyResult> to properly handle and propagate non-AttributeError exceptions - Simplify name_getter() to delegate to get_property_name() - Update format_property_error() to handle the new return type This addresses review feedback about the relationship between get_property_name() and __name__ implementation. --- vm/src/builtins/property.rs | 59 ++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 34 deletions(-) diff --git a/vm/src/builtins/property.rs b/vm/src/builtins/property.rs index 746c8f045b..1105f38bdb 100644 --- a/vm/src/builtins/property.rs +++ b/vm/src/builtins/property.rs @@ -67,20 +67,30 @@ impl GetDescriptor for PyProperty { #[pyclass(with(Constructor, Initializer, GetDescriptor), flags(BASETYPE))] impl PyProperty { // Helper method to get property name - fn get_property_name(&self, vm: &VirtualMachine) -> Option { + // Returns the name if available, None if not found, or propagates errors + fn get_property_name(&self, vm: &VirtualMachine) -> PyResult> { // First check if name was set via __set_name__ if let Some(name) = self.name.read().as_ref() { - return Some(name.clone()); + return Ok(Some(name.clone())); } // Otherwise try to get __name__ from getter - if let Some(getter) = self.getter.read().as_ref() - && let Ok(name) = getter.get_attr("__name__", vm) - { - return Some(name); + if let Some(getter) = self.getter.read().as_ref() { + match getter.get_attr("__name__", vm) { + Ok(name) => Ok(Some(name)), + Err(e) => { + // If it's an AttributeError from the getter, return None + // Otherwise, propagate the original exception (e.g., RuntimeError) + if e.class().is(vm.ctx.exceptions.attribute_error) { + Ok(None) + } else { + Err(e) + } + } + } + } else { + Ok(None) } - - None } // Descriptor methods @@ -145,31 +155,12 @@ impl PyProperty { #[pygetset(name = "__name__")] fn name_getter(&self, vm: &VirtualMachine) -> PyResult { - // If name was explicitly set via __set_name__ or direct assignment - // (even to None), return it as-is - if let Some(name) = self.name.read().as_ref() { - return Ok(name.clone()); - } - - // Fallback to getter's __name__ if available - if let Some(getter) = self.getter.read().as_ref() { - match getter.get_attr("__name__", vm) { - Ok(name) => Ok(name), - Err(e) => { - // If it's an AttributeError from the getter, convert to property's AttributeError - // Otherwise, propagate the original exception (e.g., RuntimeError) - if e.class().is(vm.ctx.exceptions.attribute_error) { - return Err(vm.new_attribute_error( - "'property' object has no attribute '__name__'".to_owned(), - )); - } - - Err(e) - } - } - } else { - // If no getter, raise AttributeError - Err(vm.new_attribute_error("'property' object has no attribute '__name__'".to_owned())) + // Use get_property_name helper to get the name + match self.get_property_name(vm)? { + Some(name) => Ok(name), + None => Err( + vm.new_attribute_error("'property' object has no attribute '__name__'".to_owned()) + ), } } @@ -323,7 +314,7 @@ impl PyProperty { error_type: &str, vm: &VirtualMachine, ) -> PyResult { - let prop_name = self.get_property_name(vm); + let prop_name = self.get_property_name(vm)?; let obj_type = obj.class(); let qualname = obj_type.__qualname__(vm); From d04d121a7d7e1cc2fdb075c9b10626e06b091228 Mon Sep 17 00:00:00 2001 From: Jiseok CHOI Date: Sat, 1 Nov 2025 12:07:11 +0900 Subject: [PATCH 3/3] style comment --- vm/src/builtins/property.rs | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/vm/src/builtins/property.rs b/vm/src/builtins/property.rs index 1105f38bdb..a75357f761 100644 --- a/vm/src/builtins/property.rs +++ b/vm/src/builtins/property.rs @@ -74,22 +74,22 @@ impl PyProperty { return Ok(Some(name.clone())); } - // Otherwise try to get __name__ from getter - if let Some(getter) = self.getter.read().as_ref() { - match getter.get_attr("__name__", vm) { - Ok(name) => Ok(Some(name)), - Err(e) => { - // If it's an AttributeError from the getter, return None - // Otherwise, propagate the original exception (e.g., RuntimeError) - if e.class().is(vm.ctx.exceptions.attribute_error) { - Ok(None) - } else { - Err(e) - } + let getter = self.getter.read(); + let Some(getter) = getter.as_ref() else { + return Ok(None); + }; + + match getter.get_attr("__name__", vm) { + Ok(name) => Ok(Some(name)), + Err(e) => { + // If it's an AttributeError from the getter, return None + // Otherwise, propagate the original exception (e.g., RuntimeError) + if e.class().is(vm.ctx.exceptions.attribute_error) { + Ok(None) + } else { + Err(e) } } - } else { - Ok(None) } } @@ -155,7 +155,6 @@ impl PyProperty { #[pygetset(name = "__name__")] fn name_getter(&self, vm: &VirtualMachine) -> PyResult { - // Use get_property_name helper to get the name match self.get_property_name(vm)? { Some(name) => Ok(name), None => Err(