Skip to content

Commit a09b860

Browse files
committed
Narrow to true but keep the isinstance call
1 parent 1e9b95f commit a09b860

File tree

3 files changed

+22
-14
lines changed

3 files changed

+22
-14
lines changed

Lib/test/test_capi/test_opt.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2234,7 +2234,8 @@ def testfunc(n):
22342234
x = 0
22352235
for _ in range(n):
22362236
# One of the classes is unknown, but it comes
2237-
# after a known class, so we can narrow to True
2237+
# after a known class, so we can narrow to True and
2238+
# remove the isinstance call.
22382239
y = isinstance(42, (int, eval('str')))
22392240
if y:
22402241
x += 1
@@ -2246,16 +2247,17 @@ def testfunc(n):
22462247
uops = get_opnames(ex)
22472248
self.assertNotIn("_CALL_ISINSTANCE", uops)
22482249
self.assertNotIn("_TO_BOOL_BOOL", uops)
2249-
self.assertNotIn("_GUARD_IS_FALSE_POP", uops)
2250+
self.assertNotIn("_GUARD_IS_TRUE_POP", uops)
22502251
self.assertIn("_BUILD_TUPLE", uops)
22512252
self.assertIn("_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW", uops)
22522253

22532254
def test_call_isinstance_tuple_of_classes_true_unknown_2(self):
22542255
def testfunc(n):
22552256
x = 0
22562257
for _ in range(n):
2257-
# One of the classes is unknown, so we can't narrow
2258-
# to True or False, only bool
2258+
# We can narrow to True, but since the unknown class comes
2259+
# first and could potentially trigger an __instancecheck__,
2260+
# we can't remove the isinstance call.
22592261
y = isinstance(42, (eval('str'), int))
22602262
if y:
22612263
x += 1
@@ -2267,14 +2269,13 @@ def testfunc(n):
22672269
uops = get_opnames(ex)
22682270
self.assertIn("_CALL_ISINSTANCE", uops)
22692271
self.assertNotIn("_TO_BOOL_BOOL", uops)
2270-
self.assertIn("_GUARD_IS_TRUE_POP", uops)
2272+
self.assertNotIn("_GUARD_IS_TRUE_POP", uops)
22712273

22722274
def test_call_isinstance_tuple_of_classes_true_unknown_3(self):
22732275
def testfunc(n):
22742276
x = 0
22752277
for _ in range(n):
2276-
# One of the classes is unknown, so we can't narrow
2277-
# to True or False, only bool
2278+
# We can only narrow to bool here
22782279
y = isinstance(42, (str, eval('int')))
22792280
if y:
22802281
x += 1
@@ -2292,8 +2293,7 @@ def test_call_isinstance_tuple_of_classes_true_unknown_4(self):
22922293
def testfunc(n):
22932294
x = 0
22942295
for _ in range(n):
2295-
# One of the classes is unknown, so we can't narrow
2296-
# to True or False, only bool
2296+
# We can only narrow to bool here
22972297
y = isinstance(42, (eval('int'), str))
22982298
if y:
22992299
x += 1

Python/optimizer_bytecodes.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -973,15 +973,17 @@ dummy_func(void) {
973973
int length = sym_tuple_length(cls);
974974
if (length != -1) {
975975
// We cannot do anything about tuples with unknown length
976+
bool can_replace_op = true;
976977
PyObject *out = Py_False;
977978
for (int i = 0; i < length; i++) {
978979
JitOptRef item = sym_tuple_getitem(ctx, cls, i);
979980
if (!sym_has_type(item)) {
980981
// There is an unknown item in the tuple.
981982
// It could potentially define its own __instancecheck__
982-
// method so we can only deduce bool.
983+
// so it is no longer possible to replace the op with a const load.
983984
out = NULL;
984-
break;
985+
can_replace_op = false;
986+
continue;
985987
}
986988
PyTypeObject *cls_o = (PyTypeObject *)sym_get_const(ctx, item);
987989
if (cls_o &&
@@ -994,7 +996,9 @@ dummy_func(void) {
994996
}
995997
if (out) {
996998
sym_set_const(res, out);
997-
REPLACE_OP(this_instr, _POP_CALL_TWO_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)out);
999+
if (can_replace_op) {
1000+
REPLACE_OP(this_instr, _POP_CALL_TWO_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)out);
1001+
}
9981002
}
9991003
}
10001004
}

Python/optimizer_cases.c.h

Lines changed: 6 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)