From d42d048c70ebd4166db4e14ce84fccb71165a409 Mon Sep 17 00:00:00 2001 From: jfh Date: Sat, 28 Aug 2021 20:57:34 +0300 Subject: [PATCH] Clean up edge cases in isinstance and issubclass. --- Lib/test/test_isinstance.py | 26 +------------ vm/src/vm.rs | 74 ++++++++++++++++++++++++++++--------- 2 files changed, 58 insertions(+), 42 deletions(-) diff --git a/Lib/test/test_isinstance.py b/Lib/test/test_isinstance.py index a2e95793c..f8e025c9f 100644 --- a/Lib/test/test_isinstance.py +++ b/Lib/test/test_isinstance.py @@ -22,8 +22,6 @@ class TestIsInstanceExceptions(unittest.TestCase): # Sounds complicated, I know, but this mimics a situation where an # extension type raises an AttributeError when its __bases__ attribute is # gotten. In that case, isinstance() should return False. - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_class_has_no_bases(self): class I(object): def getclass(self): @@ -40,8 +38,6 @@ class TestIsInstanceExceptions(unittest.TestCase): # Like above except that inst.__class__.__bases__ raises an exception # other than AttributeError - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_bases_raises_other_than_attribute_error(self): class E(object): def getbases(self): @@ -62,8 +58,6 @@ class TestIsInstanceExceptions(unittest.TestCase): # Here's a situation where getattr(cls, '__bases__') raises an exception. # If that exception is not AttributeError, it should not get masked - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_dont_mask_non_attribute_error(self): class I: pass @@ -88,8 +82,6 @@ class TestIsInstanceExceptions(unittest.TestCase): # check that we don't mask non AttributeErrors # see: http://bugs.python.org/issue1574217 - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_isinstance_dont_mask_non_attribute_error(self): class C(object): def getclass(self): @@ -108,8 +100,6 @@ class TestIsInstanceExceptions(unittest.TestCase): # issubclass() instead of isinstance() -- really PyObject_IsSubclass() # vs. PyObject_IsInstance(). class TestIsSubclassExceptions(unittest.TestCase): - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_dont_mask_non_attribute_error(self): class C(object): def getbases(self): @@ -134,8 +124,6 @@ class TestIsSubclassExceptions(unittest.TestCase): # second arg (the cls arg) is tested. This means the first arg must # return a valid __bases__, and it's okay for it to be a normal -- # unrelated by inheritance -- class. - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_dont_mask_non_attribute_error_in_cls_arg(self): class B: pass @@ -198,8 +186,6 @@ class TestIsInstanceIsSubclass(unittest.TestCase): # 'abstract' classes and instances. This case tries to test all # combinations. - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_isinstance_normal(self): # normal instances self.assertEqual(True, isinstance(Super(), Super)) @@ -210,8 +196,6 @@ class TestIsInstanceIsSubclass(unittest.TestCase): self.assertEqual(True, isinstance(Child(), Super)) self.assertEqual(False, isinstance(Child(), AbstractSuper)) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_isinstance_abstract(self): # abstract instances self.assertEqual(True, isinstance(AbstractSuper(), AbstractSuper)) @@ -234,8 +218,6 @@ class TestIsInstanceIsSubclass(unittest.TestCase): self.assertEqual(True, issubclass(Child, Super)) self.assertEqual(False, issubclass(Child, AbstractSuper)) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_subclass_abstract(self): # abstract classes self.assertEqual(True, issubclass(AbstractSuper, AbstractSuper)) @@ -259,22 +241,20 @@ class TestIsInstanceIsSubclass(unittest.TestCase): self.assertEqual(True, issubclass(int, (int, (float, int)))) self.assertEqual(True, issubclass(str, (str, (Child, str)))) - # TODO: RUSTPYTHON + # TODO: RUSTPYTHON requires issue 2680 for stack depth detection in Rust @unittest.expectedFailure def test_subclass_recursion_limit(self): # make sure that issubclass raises RecursionError before the C stack is # blown self.assertRaises(RecursionError, blowstack, issubclass, str, str) - # TODO: RUSTPYTHON + # TODO: RUSTPYTHON requires issue 2680 for stack depth detection in Rust @unittest.expectedFailure def test_isinstance_recursion_limit(self): # make sure that issubclass raises RecursionError before the C stack is # blown self.assertRaises(RecursionError, blowstack, isinstance, '', str) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_issubclass_refcount_handling(self): # bpo-39382: abstract_issubclass() didn't hold item reference while # peeking in the bases tuple, in the single inheritance case. @@ -295,8 +275,6 @@ class TestIsInstanceIsSubclass(unittest.TestCase): self.assertEqual(True, issubclass(B(), int)) - # TODO: RUSTPYTHON - @unittest.expectedFailure @unittest.skipIf(sys.platform == "win32", "thread 'main' has overflowed its stack") def test_infinite_recursion_in_bases(self): class X: diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 78d1192ef..3e2cedac8 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -944,14 +944,52 @@ impl VirtualMachine { } } - pub fn _isinstance(&self, obj: &PyObjectRef, cls: &PyObjectRef) -> PyResult { + // Equivalent to check_class. Masks Attribute errors (into TypeErrors) and lets everything + // else go through. + fn check_cls(&self, cls: &PyObjectRef, msg: F) -> PyResult + where + F: Fn() -> String, + { + self.get_attribute(cls.clone(), "__bases__").map_err(|e| { + // Only mask AttributeErrors. + if e.class().is(&self.ctx.exceptions.attribute_error) { + self.new_type_error(msg()) + } else { + e + } + }) + } + + fn abstract_isinstance(&self, obj: &PyObjectRef, cls: &PyObjectRef) -> PyResult { if let Ok(typ) = PyTypeRef::try_from_object(self, cls.clone()) { - Ok(obj.isinstance(&typ)) + if obj.class().issubclass(typ.clone()) { + Ok(true) + } else if let Ok(icls) = + PyTypeRef::try_from_object(self, self.get_attribute(obj.clone(), "__class__")?) + { + if icls.is(&obj.class()) { + Ok(false) + } else { + Ok(icls.issubclass(typ)) + } + } else { + Ok(false) + } } else { - Err(self.new_type_error(format!( - "isinstance() arg 2 must be a type or tuple of types, not {}", - cls.class() - ))) + self.check_cls(cls, || { + format!( + "isinstance() arg 2 must be a type or tuple of types, not {}", + cls.class() + ) + }) + .and_then(|_| { + let icls = self.get_attribute(obj.clone(), "__class__")?; + if self.is_none(&icls) { + Ok(false) + } else { + self.abstract_issubclass(icls, cls) + } + }) } } @@ -965,7 +1003,7 @@ impl VirtualMachine { } if cls.class().is(&self.ctx.types.type_type) { - return self._isinstance(obj, cls); + return self.abstract_isinstance(obj, cls); } if let Ok(tuple) = PyTupleRef::try_from_object(self, cls.clone()) { @@ -982,13 +1020,13 @@ impl VirtualMachine { return pybool::boolval(self, ret); } - self._isinstance(obj, cls) + self.abstract_isinstance(obj, cls) } fn abstract_issubclass(&self, subclass: PyObjectRef, cls: &PyObjectRef) -> PyResult { let mut derived = subclass; loop { - if derived.class().is(cls) { + if derived.is(cls) { return Ok(true); } @@ -1024,19 +1062,19 @@ impl VirtualMachine { ) { Ok(subclass.issubclass(cls)) } else { - if self.get_attribute(subclass.clone(), "__bases__").is_err() { - return Err(self.new_type_error(format!( + self.check_cls(subclass, || { + 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!( + ) + }) + .and(self.check_cls(cls, || { + format!( "issubclass() arg 2 must be a class or tuple of classes, not {}", cls.class() - ))); - } - self.abstract_issubclass(subclass.clone(), cls) + ) + })) + .and(self.abstract_issubclass(subclass.clone(), cls)) } }