Bail when encountering an unknown class

Author: tomasr8

Lib/test/test_capi/test_opt.py

@@ -2143,13 +2143,13 @@ def testfunc(n):
         self.assertIn("_BUILD_TUPLE", uops)
         self.assertIn("_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW", uops)
 
-    def test_call_isinstance_tuple_of_classes_true_unknown(self):
+    def test_call_isinstance_tuple_of_classes_true_unknown_1(self):
         def testfunc(n):
             x = 0
             for _ in range(n):
-                # One of the classes is unknown, but we can still
-                # narrow to True
-                y = isinstance(42, (eval('str'), int))
+                # One of the classes is unknown, but it comes
+                # after a known class, so we can narrow to True
+                y = isinstance(42, (int, eval('str')))
                 if y:
                     x += 1
             return x
@@ -2160,11 +2160,30 @@ def testfunc(n):
         uops = get_opnames(ex)
         self.assertNotIn("_CALL_ISINSTANCE", uops)
         self.assertNotIn("_TO_BOOL_BOOL", uops)
-        self.assertNotIn("_GUARD_IS_TRUE_POP", uops)
+        self.assertNotIn("_GUARD_IS_FALSE_POP", uops)
         self.assertIn("_BUILD_TUPLE", uops)
         self.assertIn("_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW", uops)
 
-    def test_call_isinstance_tuple_of_classes_unknown_not_narrowed(self):
+    def test_call_isinstance_tuple_of_classes_true_unknown_2(self):
+        def testfunc(n):
+            x = 0
+            for _ in range(n):
+                # One of the classes is unknown, so we can't narrow
+                # to True or False, only bool
+                y = isinstance(42, (eval('str'), int))
+                if y:
+                    x += 1
+            return x
+
+        res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
+        self.assertEqual(res, TIER2_THRESHOLD)
+        self.assertIsNotNone(ex)
+        uops = get_opnames(ex)
+        self.assertIn("_CALL_ISINSTANCE", uops)
+        self.assertNotIn("_TO_BOOL_BOOL", uops)
+        self.assertIn("_GUARD_IS_TRUE_POP", uops)
+
+    def test_call_isinstance_tuple_of_classes_true_unknown_3(self):
         def testfunc(n):
             x = 0
             for _ in range(n):
@@ -2183,6 +2202,25 @@ def testfunc(n):
         self.assertNotIn("_TO_BOOL_BOOL", uops)
         self.assertIn("_GUARD_IS_TRUE_POP", uops)
 
+    def test_call_isinstance_tuple_of_classes_true_unknown_4(self):
+        def testfunc(n):
+            x = 0
+            for _ in range(n):
+                # One of the classes is unknown, so we can't narrow
+                # to True or False, only bool
+                y = isinstance(42, (eval('int'), str))
+                if y:
+                    x += 1
+            return x
+
+        res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
+        self.assertEqual(res, TIER2_THRESHOLD)
+        self.assertIsNotNone(ex)
+        uops = get_opnames(ex)
+        self.assertIn("_CALL_ISINSTANCE", uops)
+        self.assertNotIn("_TO_BOOL_BOOL", uops)
+        self.assertIn("_GUARD_IS_TRUE_POP", uops)
+
     def test_call_isinstance_empty_tuple(self):
         def testfunc(n):
             x = 0
@@ -2248,6 +2286,36 @@ def testfunc(n):
         self.assertNotIn("_TO_BOOL_BOOL", uops)
         self.assertIn("_GUARD_IS_TRUE_POP", uops)
 
+    def test_call_isinstance_tuple_metaclass(self):
+        calls = 0
+
+        class Meta(type):
+            def __instancecheck__(self, _):
+                nonlocal calls
+                calls += 1
+                return False
+
+        class Unknown(metaclass=Meta):
+            pass
+
+        def testfunc(n):
+            x = 0
+            for _ in range(n):
+                # Only narrowed to bool
+                y = isinstance(42, (Unknown, int))
+                if y:
+                    x += 1
+            return x, calls
+
+        (res, calls), ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
+        self.assertEqual(res, TIER2_THRESHOLD)
+        self.assertEqual(calls, TIER2_THRESHOLD)
+        self.assertIsNotNone(ex)
+        uops = get_opnames(ex)
+        self.assertIn("_CALL_ISINSTANCE", uops)
+        self.assertNotIn("_TO_BOOL_BOOL", uops)
+        self.assertIn("_GUARD_IS_TRUE_POP", uops)
+
     def test_set_type_version_sets_type(self):
         class C:
             A = 1

Python/optimizer_bytecodes.c

@@ -938,9 +938,6 @@ dummy_func(void) {
     }
 
     op(_CALL_ISINSTANCE, (unused, unused, instance, cls -- res)) {
-// The below define is equivalent to PyObject_TypeCheck(inst, cls)
-#define sym_IS_SUBTYPE(inst, cls) ((inst) == (cls) || PyType_IsSubtype(inst, cls))
-
         // the result is always a bool, but sometimes we can
         // narrow it down to True or False
         res = sym_new_type(ctx, &PyBool_Type);
@@ -951,7 +948,7 @@ dummy_func(void) {
             // known types, meaning we can deduce either True or False
 
             PyObject *out = Py_False;
-            if (sym_IS_SUBTYPE(inst_type, cls_o)) {
+            if (inst_type == cls_o || PyType_IsSubtype(inst_type, cls_o)) {
                 out = Py_True;
             }
             sym_set_const(res, out);
@@ -972,23 +969,24 @@ dummy_func(void) {
                 for (int i = 0; i < length; i++) {
                     JitOptSymbol *item = sym_tuple_getitem(ctx, cls, i);
                     if (!sym_has_type(item)) {
-                        // There is an unknown item in the tuple,
-                        // we can no longer deduce False.
+                        // There is an unknown item in the tuple.
+                        // It could potentially define its own __instancecheck__
+                        // method so we can only deduce bool.
                         all_items_known = false;
-                        continue;
+                        break;
                     }
                     PyTypeObject *cls_o = (PyTypeObject *)sym_get_const(ctx, item);
                     if (cls_o &&
                         sym_matches_type(item, &PyType_Type) &&
-                        sym_IS_SUBTYPE(inst_type, cls_o))
+                        (inst_type == cls_o || PyType_IsSubtype(inst_type, cls_o)))
                     {
                         out = Py_True;
                         break;
                     }
                 }
-                if (!out && all_items_known) {
+                if (out == NULL && all_items_known) {
                     // We haven't deduced True, but all items in the tuple are known
-                    // so we can deduce False
+                    // so we can deduce False.
                     out = Py_False;
                 }
                 if (out) {
@@ -997,7 +995,6 @@ dummy_func(void) {
                 }
             }
         }
-#undef sym_IS_SUBTYPE
     }
 
     op(_GUARD_IS_TRUE_POP, (flag -- )) {