From 1ddeb67ad0e61c9cfc8508b0ad7af98fa5e7ebd4 Mon Sep 17 00:00:00 2001 From: snowapril Date: Wed, 25 Aug 2021 22:36:08 +0900 Subject: [PATCH 1/4] builtins: fix issubclass method miswork This commit fix issue #2943. In cpython, both first and second argument can be value (not type). As second parameter `cls` is defined as PyTypeRef, `vm.call_special_method` use `subclasscheck` method in `PyTuple`, never could reach to user defined `subclasscheck` method. In general, first argument of `issubclass` must be ``, but if second argument define `__subclasscheck__`, rustpython must test it first. Therefore, for first argument enable to have different type, I switch it's type as `PyObjectRef`. Signed-off-by: snowapril --- vm/src/builtins/make_module.rs | 18 ++----- vm/src/vm.rs | 85 +++++++++++++++++++++++++++++++--- 2 files changed, 81 insertions(+), 22 deletions(-) diff --git a/vm/src/builtins/make_module.rs b/vm/src/builtins/make_module.rs index 5bb858b05..73c87bf69 100644 --- a/vm/src/builtins/make_module.rs +++ b/vm/src/builtins/make_module.rs @@ -25,9 +25,7 @@ mod decl { use crate::common::{hash::PyHash, str::to_ascii}; #[cfg(feature = "rustpython-compiler")] use crate::compile; - use crate::function::{ - single_or_tuple_any, Args, FuncArgs, KwArgs, OptionalArg, OptionalOption, - }; + use crate::function::{Args, FuncArgs, KwArgs, OptionalArg, OptionalOption}; use crate::iterator; use crate::readline::{Readline, ReadlineResult}; use crate::scope::Scope; @@ -412,18 +410,8 @@ mod decl { } #[pyfunction] - fn issubclass(subclass: PyTypeRef, typ: PyObjectRef, vm: &VirtualMachine) -> PyResult { - single_or_tuple_any( - typ, - &|cls: &PyTypeRef| vm.issubclass(&subclass, cls), - &|o| { - format!( - "issubclass() arg 2 must be a class or tuple of classes, not {}", - o.class() - ) - }, - vm, - ) + fn issubclass(subclass: PyObjectRef, typ: PyObjectRef, vm: &VirtualMachine) -> PyResult { + vm.issubclass(&subclass, &typ) } #[pyfunction] diff --git a/vm/src/vm.rs b/vm/src/vm.rs index d8067d24f..78d1192ef 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -985,15 +985,86 @@ impl VirtualMachine { self._isinstance(obj, cls) } + fn abstract_issubclass(&self, subclass: PyObjectRef, cls: &PyObjectRef) -> PyResult { + let mut derived = subclass; + loop { + if derived.class().is(cls) { + return Ok(true); + } + + let bases = self.get_attribute(derived, "__bases__")?; + let tuple = PyTupleRef::try_from_object(self, bases)?; + + let n = tuple.len(); + match n { + 0 => { + return Ok(false); + } + 1 => { + derived = tuple.fast_getitem(0); + continue; + } + _ => { + for i in 0..n { + if let Ok(true) = self.abstract_issubclass(tuple.fast_getitem(i), cls) { + return Ok(true); + } + } + } + } + + return Ok(false); + } + } + + fn recursive_issubclass(&self, subclass: &PyObjectRef, cls: &PyObjectRef) -> PyResult { + if let (Ok(subclass), Ok(cls)) = ( + PyTypeRef::try_from_object(self, subclass.clone()), + PyTypeRef::try_from_object(self, cls.clone()), + ) { + Ok(subclass.issubclass(cls)) + } else { + if self.get_attribute(subclass.clone(), "__bases__").is_err() { + return Err(self.new_type_error(format!( + "issubclass() arg 1 must be a class, not {}", + subclass.class() + ))); + } + if self.get_attribute(cls.clone(), "__bases__").is_err() { + return Err(self.new_type_error(format!( + "issubclass() arg 2 must be a class or tuple of classes, not {}", + cls.class() + ))); + } + self.abstract_issubclass(subclass.clone(), cls) + } + } + /// Determines if `subclass` is a subclass of `cls`, either directly, indirectly or virtually /// via the __subclasscheck__ magic method. - pub fn issubclass(&self, subclass: &PyTypeRef, cls: &PyTypeRef) -> PyResult { - let ret = self.call_special_method( - cls.as_object().clone(), - "__subclasscheck__", - (subclass.clone(),), - )?; - pybool::boolval(self, ret) + pub fn issubclass(&self, subclass: &PyObjectRef, cls: &PyObjectRef) -> PyResult { + if cls.class().is(&self.ctx.types.type_type) { + if subclass.is(cls) { + return Ok(true); + } + return self.recursive_issubclass(subclass, cls); + } + + if let Ok(tuple) = PyTupleRef::try_from_object(self, cls.clone()) { + for typ in tuple.as_slice().iter() { + if self.issubclass(subclass, typ)? { + return Ok(true); + } + } + return Ok(false); + } + + if let Ok(meth) = self.get_special_method(cls.clone(), "__subclasscheck__")? { + let ret = meth.invoke((subclass.clone(),), self)?; + return pybool::boolval(self, ret); + } + + self.recursive_issubclass(subclass, cls) } pub fn call_get_descriptor_specific( From 323b5f7c22fd2763dee9cb7b063313fab73de8e9 Mon Sep 17 00:00:00 2001 From: snowapril Date: Wed, 25 Aug 2021 22:57:26 +0900 Subject: [PATCH 2/4] add extra test test issubclass on general type Signed-off-by: snowapril --- extra_tests/snippets/issubclass.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/extra_tests/snippets/issubclass.py b/extra_tests/snippets/issubclass.py index 62577f213..7f1d87abb 100644 --- a/extra_tests/snippets/issubclass.py +++ b/extra_tests/snippets/issubclass.py @@ -49,6 +49,13 @@ assert issubclass(AlwaysSubClass, AlwaysSubClass) assert issubclass(InheritedAlwaysSubClass, AlwaysSubClass) assert issubclass(AlwaysSubClass, InheritedAlwaysSubClass) +class GenericInstance: + def __subclasscheck__(self, _): + return True + +assert issubclass(A, GenericInstance()) +assert issubclass(list, GenericInstance()) +assert issubclass([], GenericInstance()) class MCAVirtualSubClass(type): def __subclasscheck__(self, subclass): From 535f0734a01b929a7b6aebf89d29137b6755a996 Mon Sep 17 00:00:00 2001 From: snowapril Date: Fri, 27 Aug 2021 09:45:34 +0900 Subject: [PATCH 3/4] remove expected_failure annotation Signed-off-by: snowapril --- Lib/test/test_isinstance.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_isinstance.py b/Lib/test/test_isinstance.py index 5834e0884..ad8fcd3b5 100644 --- a/Lib/test/test_isinstance.py +++ b/Lib/test/test_isinstance.py @@ -224,8 +224,6 @@ class TestIsInstanceIsSubclass(unittest.TestCase): self.assertEqual(False, isinstance(AbstractChild(), Super)) self.assertEqual(False, isinstance(AbstractChild(), Child)) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_subclass_normal(self): # normal classes self.assertEqual(True, issubclass(Super, Super)) From e208138d6232e9c8c3657bc47dcd485647067a75 Mon Sep 17 00:00:00 2001 From: snowapril Date: Fri, 27 Aug 2021 20:04:45 +0900 Subject: [PATCH 4/4] skip test_infinite_recursion_in_bases test in win32 Signed-off-by: snowapril --- Lib/test/test_isinstance.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_isinstance.py b/Lib/test/test_isinstance.py index ad8fcd3b5..a2e95793c 100644 --- a/Lib/test/test_isinstance.py +++ b/Lib/test/test_isinstance.py @@ -297,6 +297,7 @@ class TestIsInstanceIsSubclass(unittest.TestCase): # TODO: RUSTPYTHON @unittest.expectedFailure + @unittest.skipIf(sys.platform == "win32", "thread 'main' has overflowed its stack") def test_infinite_recursion_in_bases(self): class X: @property