From 4409460cb3012074c2f96c582b8906f27aa93474 Mon Sep 17 00:00:00 2001 From: Chris Moradi <37349208+chrismoradi@users.noreply.github.com> Date: Sun, 28 Nov 2021 15:00:32 -0800 Subject: [PATCH 1/3] Fix __isabstractmethod__ for class/static methods and properties Includes a minor departure from CPython implementation of inspect.isabstract because TPFLAGS_IS_ABSTRACT is not set in obj.__flags__. --- Lib/inspect.py | 6 +++--- Lib/test/test_abc.py | 12 ------------ vm/src/builtins/classmethod.rs | 14 ++++++++++++++ vm/src/builtins/property.rs | 26 ++++++++++++++++++++++++++ vm/src/builtins/staticmethod.rs | 16 +++++++++++++++- 5 files changed, 58 insertions(+), 16 deletions(-) diff --git a/Lib/inspect.py b/Lib/inspect.py index 3ff395ca3..67fb9edca 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -300,16 +300,16 @@ def isroutine(object): def isabstract(object): """Return true if the object is an abstract base class (ABC).""" + # TODO: RUSTPYTHON + # TPFLAGS_IS_ABSTRACT is not being set for abstract classes, so this implementation differs from CPython. if not isinstance(object, type): return False - if object.__flags__ & TPFLAGS_IS_ABSTRACT: - return True if not issubclass(type(object), abc.ABCMeta): return False if hasattr(object, '__abstractmethods__'): # It looks like ABCMeta.__new__ has finished running; # TPFLAGS_IS_ABSTRACT should have been accurate. - return False + return bool(getattr(object, '__abstractmethods__')) # It looks like ABCMeta.__new__ has not finished running yet; we're # probably in __init_subclass__. We'll look for abstractmethods manually. for name, value in object.__dict__.items(): diff --git a/Lib/test/test_abc.py b/Lib/test/test_abc.py index 5b897ba8c..ea553b6c5 100644 --- a/Lib/test/test_abc.py +++ b/Lib/test/test_abc.py @@ -71,8 +71,6 @@ def test_factory(abc_ABCMeta, abc_get_cache_token): class TestABC(unittest.TestCase): - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_ABC_helper(self): # create an ABC using the helper class and perform basic checks class C(abc.ABC): @@ -93,8 +91,6 @@ def test_factory(abc_ABCMeta, abc_get_cache_token): def bar(self): pass self.assertFalse(hasattr(bar, "__isabstractmethod__")) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_abstractproperty_basics(self): @property @abc.abstractmethod @@ -113,8 +109,6 @@ def test_factory(abc_ABCMeta, abc_get_cache_token): def foo(self): return super().foo self.assertEqual(D().foo, 3) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_abstractclassmethod_basics(self): @classmethod @abc.abstractmethod @@ -135,8 +129,6 @@ def test_factory(abc_ABCMeta, abc_get_cache_token): self.assertEqual(D.foo(), 'D') self.assertEqual(D().foo(), 'D') - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_abstractstaticmethod_basics(self): @staticmethod @abc.abstractmethod @@ -157,8 +149,6 @@ def test_factory(abc_ABCMeta, abc_get_cache_token): self.assertEqual(D.foo(), 4) self.assertEqual(D().foo(), 4) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_abstractmethod_integration(self): for abstractthing in [abc.abstractmethod, abc.abstractproperty, abc.abstractclassmethod, @@ -187,8 +177,6 @@ def test_factory(abc_ABCMeta, abc_get_cache_token): self.assertRaises(TypeError, F) # because bar is abstract now self.assertTrue(isabstract(F)) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_descriptors_with_abstractmethod(self): class C(metaclass=abc_ABCMeta): @property diff --git a/vm/src/builtins/classmethod.rs b/vm/src/builtins/classmethod.rs index ae115b1e4..553f6f49c 100644 --- a/vm/src/builtins/classmethod.rs +++ b/vm/src/builtins/classmethod.rs @@ -76,6 +76,20 @@ impl PyClassMethod { fn func(&self) -> PyObjectRef { self.callable.clone() } + + #[pyproperty(magic)] + fn isabstractmethod(&self, vm: &VirtualMachine) -> PyObjectRef { + match vm.get_attribute_opt(self.callable.clone(), "__isabstractmethod__") { + Ok(Some(is_abstract)) => is_abstract, + _ => vm.ctx.new_bool(false).into(), + } + } + + #[pyproperty(magic, setter)] + fn set_isabstractmethod(&self, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + self.callable.set_attr("__isabstractmethod__", value, vm)?; + Ok(()) + } } pub(crate) fn init(context: &PyContext) { diff --git a/vm/src/builtins/property.rs b/vm/src/builtins/property.rs index ee7f51dcd..c2761805f 100644 --- a/vm/src/builtins/property.rs +++ b/vm/src/builtins/property.rs @@ -217,6 +217,32 @@ impl PyProperty { } .into_ref_with_type(vm, TypeProtocol::clone_class(&zelf)) } + + #[pyproperty(magic)] + fn isabstractmethod(&self, vm: &VirtualMachine) -> PyObjectRef { + let getter_abstract = match self.getter.read().to_owned() { + Some(getter) => getter + .get_attr("__isabstractmethod__", vm) + .unwrap_or(vm.ctx.new_bool(false).into()), + _ => vm.ctx.new_bool(false).into(), + }; + let setter_abstract = match self.setter.read().to_owned() { + Some(setter) => setter + .get_attr("__isabstractmethod__", vm) + .unwrap_or(vm.ctx.new_bool(false).into()), + _ => vm.ctx.new_bool(false).into(), + }; + vm._or(&setter_abstract, &getter_abstract) + .unwrap_or(vm.ctx.new_bool(false).into()) + } + + #[pyproperty(magic, setter)] + fn set_isabstractmethod(&self, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + if let Some(getter) = self.getter.read().to_owned() { + getter.set_attr("__isabstractmethod__", value, vm)?; + } + Ok(()) + } } pub(crate) fn init(context: &PyContext) { diff --git a/vm/src/builtins/staticmethod.rs b/vm/src/builtins/staticmethod.rs index a6e7b8938..038b93b16 100644 --- a/vm/src/builtins/staticmethod.rs +++ b/vm/src/builtins/staticmethod.rs @@ -60,7 +60,21 @@ impl PyStaticMethod { } #[pyimpl(with(GetDescriptor, Constructor), flags(BASETYPE, HAS_DICT))] -impl PyStaticMethod {} +impl PyStaticMethod { + #[pyproperty(magic)] + fn isabstractmethod(&self, vm: &VirtualMachine) -> PyObjectRef { + match vm.get_attribute_opt(self.callable.clone(), "__isabstractmethod__") { + Ok(Some(is_abstract)) => is_abstract, + _ => vm.ctx.new_bool(false).into(), + } + } + + #[pyproperty(magic, setter)] + fn set_isabstractmethod(&self, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + self.callable.set_attr("__isabstractmethod__", value, vm)?; + Ok(()) + } +} pub fn init(context: &PyContext) { PyStaticMethod::extend_class(context, &context.types.staticmethod_type); From 5fef0267af9ae48cf7f2e5e5bb394f4a9a895f5e Mon Sep 17 00:00:00 2001 From: Chris Moradi <37349208+chrismoradi@users.noreply.github.com> Date: Sun, 28 Nov 2021 15:43:50 -0800 Subject: [PATCH 2/3] Fix clippy issue --- vm/src/builtins/property.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vm/src/builtins/property.rs b/vm/src/builtins/property.rs index c2761805f..585c02e15 100644 --- a/vm/src/builtins/property.rs +++ b/vm/src/builtins/property.rs @@ -223,17 +223,17 @@ impl PyProperty { let getter_abstract = match self.getter.read().to_owned() { Some(getter) => getter .get_attr("__isabstractmethod__", vm) - .unwrap_or(vm.ctx.new_bool(false).into()), + .unwrap_or_else(|_| vm.ctx.new_bool(false).into()), _ => vm.ctx.new_bool(false).into(), }; let setter_abstract = match self.setter.read().to_owned() { Some(setter) => setter .get_attr("__isabstractmethod__", vm) - .unwrap_or(vm.ctx.new_bool(false).into()), + .unwrap_or_else(|_| vm.ctx.new_bool(false).into()), _ => vm.ctx.new_bool(false).into(), }; vm._or(&setter_abstract, &getter_abstract) - .unwrap_or(vm.ctx.new_bool(false).into()) + .unwrap_or_else(|_| vm.ctx.new_bool(false).into()) } #[pyproperty(magic, setter)] From 961e7006124f62696a8e47207eabfcef28884b8c Mon Sep 17 00:00:00 2001 From: Chris Moradi <37349208+chrismoradi@users.noreply.github.com> Date: Mon, 29 Nov 2021 08:56:32 -0800 Subject: [PATCH 3/3] PR feedback: comment out original inspect.isabstract code --- Lib/inspect.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Lib/inspect.py b/Lib/inspect.py index 67fb9edca..a84f3346b 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -300,15 +300,18 @@ def isroutine(object): def isabstract(object): """Return true if the object is an abstract base class (ABC).""" - # TODO: RUSTPYTHON - # TPFLAGS_IS_ABSTRACT is not being set for abstract classes, so this implementation differs from CPython. if not isinstance(object, type): return False + # TODO: RUSTPYTHON + # TPFLAGS_IS_ABSTRACT is not being set for abstract classes, so this implementation differs from CPython. + # if object.__flags__ & TPFLAGS_IS_ABSTRACT: + # return True if not issubclass(type(object), abc.ABCMeta): return False if hasattr(object, '__abstractmethods__'): # It looks like ABCMeta.__new__ has finished running; # TPFLAGS_IS_ABSTRACT should have been accurate. + # return False return bool(getattr(object, '__abstractmethods__')) # It looks like ABCMeta.__new__ has not finished running yet; we're # probably in __init_subclass__. We'll look for abstractmethods manually.