bpo-39156: Break up COMPARE_OP into four logically distinct opcodes. (G…

…H-17754) Break up COMPARE_OP into four logically distinct opcodes: * COMPARE_OP for rich comparisons * IS_OP for 'is' and 'is not' tests * CONTAINS_OP for 'in' and 'is not' tests * JUMP_IF_NOT_EXC_MATCH for checking exceptions in 'try-except' statements.
python · Jan 14, 2020 · 9af0e47b1705457bb6b327c197f2ec5737a1d8f6 · 9af0e47
1 parent 62e3973
commit 9af0e47b1705457bb6b327c197f2ec5737a1d8f6
diff --git a/Doc/library/dis.rst b/Doc/library/dis.rst
@@ -927,6 +927,20 @@ All of the following opcodes use their arguments.
    ``cmp_op[opname]``.
 
 
+.. opcode:: IS_OP (invert)
+
+    Performs ``is`` comparison, or ``is not`` if ``invert`` is 1.
+
+   .. versionadded:: 3.9
+
+
+.. opcode:: CONTAINS_OP (invert)
+
+    Performs ``in`` comparison, or ``not in`` if ``invert`` is 1.
+
+   .. versionadded:: 3.9
+
+
 .. opcode:: IMPORT_NAME (namei)
 
    Imports the module ``co_names[namei]``.  TOS and TOS1 are popped and provide
@@ -961,6 +975,13 @@ All of the following opcodes use their arguments.
 
    .. versionadded:: 3.1
 
+.. opcode:: JUMP_IF_NOT_EXC_MATCH (target)
+
+    Tests whether the second value on the stack is an exception matching TOS,
+    and jumps if it is not. Pops two values from the stack.
+
+   .. versionadded:: 3.9
+
 
 .. opcode:: JUMP_IF_TRUE_OR_POP (target)
 

diff --git a/Include/opcode.h b/Include/opcode.h
diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py
@@ -274,6 +274,7 @@ def _write_atomic(path, data, mode=0o666):
 #     Python 3.9a0  3420 (add LOAD_ASSERTION_ERROR #34880)
 #     Python 3.9a0  3421 (simplified bytecode for with blocks #32949)
 #     Python 3.9a0  3422 (remove BEGIN_FINALLY, END_FINALLY, CALL_FINALLY, POP_FINALLY bytecodes #33387)
+#     Python 3.9a2  3423 (add IS_OP, CONTAINS_OP and JUMP_IF_NOT_EXC_MATCH bytecodes #39156)
 #
 # MAGIC must change whenever the bytecode emitted by the compiler may no
 # longer be understood by older implementations of the eval loop (usually
@@ -282,7 +283,7 @@ def _write_atomic(path, data, mode=0o666):
 # Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array
 # in PC/launcher.c must also be updated.
 
-MAGIC_NUMBER = (3422).to_bytes(2, 'little') + b'\r\n'
+MAGIC_NUMBER = (3423).to_bytes(2, 'little') + b'\r\n'
 _RAW_MAGIC_NUMBER = int.from_bytes(MAGIC_NUMBER, 'little')  # For import.c
 
 _PYCACHE = '__pycache__'

diff --git a/Lib/opcode.py b/Lib/opcode.py
@@ -21,8 +21,7 @@
 except ImportError:
     pass
 
-cmp_op = ('<', '<=', '==', '!=', '>', '>=', 'in', 'not in', 'is',
-        'is not', 'exception match', 'BAD')
+cmp_op = ('<', '<=', '==', '!=', '>', '>=')
 
 hasconst = []
 hasname = []
@@ -159,6 +158,10 @@ def jabs_op(name, op):
 
 name_op('LOAD_GLOBAL', 116)     # Index in name list
 
+def_op('IS_OP', 117)
+def_op('CONTAINS_OP', 118)
+
+jabs_op('JUMP_IF_NOT_EXC_MATCH', 121)
 jrel_op('SETUP_FINALLY', 122)   # Distance to target address
 
 def_op('LOAD_FAST', 124)        # Local variable number

diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py
diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py
@@ -65,14 +65,14 @@ def unot(x):
         self.check_lnotab(unot)
 
     def test_elim_inversion_of_is_or_in(self):
-        for line, cmp_op in (
-            ('not a is b', 'is not',),
-            ('not a in b', 'not in',),
-            ('not a is not b', 'is',),
-            ('not a not in b', 'in',),
+        for line, cmp_op, invert in (
+            ('not a is b', 'IS_OP', 1,),
+            ('not a is not b', 'IS_OP', 0,),
+            ('not a in b', 'CONTAINS_OP', 1,),
+            ('not a not in b', 'CONTAINS_OP', 0,),
             ):
             code = compile(line, '', 'single')
-            self.assertInBytecode(code, 'COMPARE_OP', cmp_op)
+            self.assertInBytecode(code, cmp_op, invert)
             self.check_lnotab(code)
 
     def test_global_as_constant(self):

diff --git a/Lib/test/test_positional_only_arg.py b/Lib/test/test_positional_only_arg.py
@@ -434,11 +434,11 @@ def g():
             def f(x: not (int is int), /): ...
 
         # without constant folding we end up with
-        # COMPARE_OP(is), UNARY_NOT
-        # with constant folding we should expect a COMPARE_OP(is not)
+        # COMPARE_OP(is), IS_OP (0)
+        # with constant folding we should expect a IS_OP (1)
         codes = [(i.opname, i.argval) for i in dis.get_instructions(g)]
         self.assertNotIn(('UNARY_NOT', None), codes)
-        self.assertIn(('COMPARE_OP', 'is not'), codes)
+        self.assertIn(('IS_OP', 1), codes)
 
 
 if __name__ == "__main__":

diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-12-30-10-53-59.bpo-39156.veT-CB.rst b/Misc/NEWS.d/next/Core and Builtins/2019-12-30-10-53-59.bpo-39156.veT-CB.rst
@@ -0,0 +1,9 @@
+Split the COMPARE_OP bytecode instruction into four distinct instructions.
+
+*  COMPARE_OP for rich comparisons
+*  IS_OP for 'is' and 'is not' tests
+*  CONTAINS_OP for 'in' and 'is not' tests
+*  JUMP_IF_NOT_EXC_MATCH for checking exceptions in 'try-except' statements.
+
+This improves the clarity of the interpreter and should provide a modest
+speedup.
diff --git a/PC/launcher.c b/PC/launcher.c
@@ -1247,6 +1247,7 @@ static PYC_MAGIC magic_values[] = {
     { 3360, 3379, L"3.6" },
     { 3390, 3399, L"3.7" },
     { 3400, 3419, L"3.8" },
+    { 3420, 3429, L"3.9" },
     { 0 }
 };
 
@@ -1830,7 +1831,7 @@ process(int argc, wchar_t ** argv)
 
 #if !defined(VENV_REDIRECT)
     /* bpo-35811: The __PYVENV_LAUNCHER__ variable is used to
-     * override sys.executable and locate the original prefix path. 
+     * override sys.executable and locate the original prefix path.
      * However, if it is silently inherited by a non-venv Python
      * process, that process will believe it is running in the venv
      * still. This is the only place where *we* can clear it (that is,

diff --git a/Python/ceval.c b/Python/ceval.c
@@ -67,7 +67,6 @@ static void maybe_dtrace_line(PyFrameObject *, int *, int *, int *);
 static void dtrace_function_entry(PyFrameObject *);
 static void dtrace_function_return(PyFrameObject *);
 
-static PyObject * cmp_outcome(PyThreadState *, int, PyObject *, PyObject *);
 static PyObject * import_name(PyThreadState *, PyFrameObject *,
                               PyObject *, PyObject *, PyObject *);
 static PyObject * import_from(PyThreadState *, PyObject *, PyObject *);
@@ -2897,19 +2896,95 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
         }
 
         case TARGET(COMPARE_OP): {
+            assert(oparg <= Py_GE);
             PyObject *right = POP();
             PyObject *left = TOP();
-            PyObject *res = cmp_outcome(tstate, oparg, left, right);
+            PyObject *res = PyObject_RichCompare(left, right, oparg);
+            SET_TOP(res);
             Py_DECREF(left);
             Py_DECREF(right);
-            SET_TOP(res);
             if (res == NULL)
                 goto error;
             PREDICT(POP_JUMP_IF_FALSE);
             PREDICT(POP_JUMP_IF_TRUE);
             DISPATCH();
         }
 
+        case TARGET(IS_OP): {
+            PyObject *right = POP();
+            PyObject *left = TOP();
+            int res = (left == right)^oparg;
+            PyObject *b = res ? Py_True : Py_False;
+            Py_INCREF(b);
+            SET_TOP(b);
+            Py_DECREF(left);
+            Py_DECREF(right);
+            PREDICT(POP_JUMP_IF_FALSE);
+            PREDICT(POP_JUMP_IF_TRUE);
+            FAST_DISPATCH();
+        }
+
+        case TARGET(CONTAINS_OP): {
+            PyObject *right = POP();
+            PyObject *left = POP();
+            int res = PySequence_Contains(right, left);
+            Py_DECREF(left);
+            Py_DECREF(right);
+            if (res < 0) {
+                goto error;
+            }
+            PyObject *b = (res^oparg) ? Py_True : Py_False;
+            Py_INCREF(b);
+            PUSH(b);
+            PREDICT(POP_JUMP_IF_FALSE);
+            PREDICT(POP_JUMP_IF_TRUE);
+            FAST_DISPATCH();
+        }
+
+#define CANNOT_CATCH_MSG "catching classes that do not inherit from "\
+                         "BaseException is not allowed"
+
+        case TARGET(JUMP_IF_NOT_EXC_MATCH): {
+            PyObject *right = POP();
+            PyObject *left = POP();
+            if (PyTuple_Check(right)) {
+                Py_ssize_t i, length;
+                length = PyTuple_GET_SIZE(right);
+                for (i = 0; i < length; i++) {
+                    PyObject *exc = PyTuple_GET_ITEM(right, i);
+                    if (!PyExceptionClass_Check(exc)) {
+                        _PyErr_SetString(tstate, PyExc_TypeError,
+                                        CANNOT_CATCH_MSG);
+                        Py_DECREF(left);
+                        Py_DECREF(right);
+                        goto error;
+                    }
+                }
+            }
+            else {
+                if (!PyExceptionClass_Check(right)) {
+                    _PyErr_SetString(tstate, PyExc_TypeError,
+                                    CANNOT_CATCH_MSG);
+                    Py_DECREF(left);
+                    Py_DECREF(right);
+                    goto error;
+                }
+            }
+            int res = PyErr_GivenExceptionMatches(left, right);
+            Py_DECREF(left);
+            Py_DECREF(right);
+            if (res > 0) {
+                /* Exception matches -- Do nothing */;
+            }
+            else if (res == 0) {
+                JUMPTO(oparg);
+            }
+            else {
+                goto error;
+            }
+            DISPATCH();
+        }
+
         case TARGET(IMPORT_NAME): {
             PyObject *name = GETITEM(names, oparg);
             PyObject *fromlist = POP();
@@ -4951,62 +5026,6 @@ _PyEval_SliceIndexNotNone(PyObject *v, Py_ssize_t *pi)
     return 1;
 }
 
-
-#define CANNOT_CATCH_MSG "catching classes that do not inherit from "\
-                         "BaseException is not allowed"
-
-static PyObject *
-cmp_outcome(PyThreadState *tstate, int op, PyObject *v, PyObject *w)
-{
-    int res = 0;
-    switch (op) {
-    case PyCmp_IS:
-        res = (v == w);
-        break;
-    case PyCmp_IS_NOT:
-        res = (v != w);
-        break;
-    case PyCmp_IN:
-        res = PySequence_Contains(w, v);
-        if (res < 0)
-            return NULL;
-        break;
-    case PyCmp_NOT_IN:
-        res = PySequence_Contains(w, v);
-        if (res < 0)
-            return NULL;
-        res = !res;
-        break;
-    case PyCmp_EXC_MATCH:
-        if (PyTuple_Check(w)) {
-            Py_ssize_t i, length;
-            length = PyTuple_Size(w);
-            for (i = 0; i < length; i += 1) {
-                PyObject *exc = PyTuple_GET_ITEM(w, i);
-                if (!PyExceptionClass_Check(exc)) {
-                    _PyErr_SetString(tstate, PyExc_TypeError,
-                                     CANNOT_CATCH_MSG);
-                    return NULL;
-                }
-            }
-        }
-        else {
-            if (!PyExceptionClass_Check(w)) {
-                _PyErr_SetString(tstate, PyExc_TypeError,
-                                 CANNOT_CATCH_MSG);
-                return NULL;
-            }
-        }
-        res = PyErr_GivenExceptionMatches(v, w);
-        break;
-    default:
-        return PyObject_RichCompare(v, w, op);
-    }
-    v = res ? Py_True : Py_False;
-    Py_INCREF(v);
-    return v;
-}
-
 static PyObject *
 import_name(PyThreadState *tstate, PyFrameObject *f,
             PyObject *name, PyObject *fromlist, PyObject *level)