Repository: MatthieuDartiailh/bytecode

Issue:
The `Compare` enum is broken starting from Python 3.9
~~~
Python 3.9.7 (default, Sep  3 2021, 12:37:55)
[Clang 12.0.5 (clang-1205.0.22.9)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from dis import cmp_op
>>> len(cmp_op)
6
>>> cmp_op
('<', '<=', '==', '!=', '>', '>=')
~~~
The [`in`](https://docs.python.org/3.9/library/dis.html#opcode-CONTAINS_OP) and [`is`](https://docs.python.org/3.9/library/dis.html#opcode-CONTAINS_OP) operators have dedicated opcodes (with an argument for the inverse) and they should be removed from `Compare`.

Discussion:
MatthieuDartiailh: Thanks for the patch. Would be possible to add a test to avoid future regression ?
P403n1x87: Yep, I'll add some tests as soon as I can. It would be great if the tests also run with every PR as I just discovered that some peephole optimisation tests are not passing with the new change and need to be adapted (as a follow up, it might be good to actually run the optimised code rather than just compare it to the expected abstract result).
MatthieuDartiailh: CIs should run on PR yes. I must have made a configuration mistake for it not to be the case or I forgot to update it after moving to the src organization.
P403n1x87: @MatthieuDartiailh this should be ready for review now
MatthieuDartiailh: @P403n1x87 Could you rebaseon main ? I fixed the CI issues.
P403n1x87: Oh I thought it was now possible to do that directly from the PR by those with merge permissions? Anyway I'll rebase this.

Patch:
diff --git a/doc/changelog.rst b/doc/changelog.rst
index 4016453..cf4b604 100644
--- a/doc/changelog.rst
+++ b/doc/changelog.rst
@@ -12,6 +12,10 @@ Bugfixes:
 - Removed ``EXC_MATCH`` from the ``Compare`` enumeration starting with Python
   3.9. The new ``JUMP_IF_NOT_EXC_MATCH`` opcode should be used instead.
 
+- Removed ``IN``, ``NOT_IN``, ``IS``, ``NOT_IS`` from the ``Compare``
+  enumeration starting with Python 3.9. The new ``CONTAINS_OP`` and ``IS_OP``
+  opcodes should be used instead.
+
 Maintenance:
 
 - Make the install process PEP517 compliant PR #97
diff --git a/src/bytecode/instr.py b/src/bytecode/instr.py
index 8ea3ab5..363a6f4 100644
--- a/src/bytecode/instr.py
+++ b/src/bytecode/instr.py
@@ -15,11 +15,11 @@ class Compare(enum.IntEnum):
     NE = 3
     GT = 4
     GE = 5
-    IN = 6
-    NOT_IN = 7
-    IS = 8
-    IS_NOT = 9
     if sys.version_info < (3, 9):
+        IN = 6
+        NOT_IN = 7
+        IS = 8
+        IS_NOT = 9
         EXC_MATCH = 10
 
 
diff --git a/src/bytecode/peephole_opt.py b/src/bytecode/peephole_opt.py
index 7216d37..1f03a63 100644
--- a/src/bytecode/peephole_opt.py
+++ b/src/bytecode/peephole_opt.py
@@ -15,12 +15,15 @@ JUMPS_ON_TRUE = frozenset(
     )
 )
 
-NOT_COMPARE = {
-    Compare.IN: Compare.NOT_IN,
-    Compare.NOT_IN: Compare.IN,
-    Compare.IS: Compare.IS_NOT,
-    Compare.IS_NOT: Compare.IS,
-}
+if sys.version_info < (3, 9):
+    NOT_COMPARE = {
+        Compare.IN: Compare.NOT_IN,
+        Compare.NOT_IN: Compare.IN,
+        Compare.IS: Compare.IS_NOT,
+        Compare.IS_NOT: Compare.IS,
+    }
+else:
+    NOT_COMPARE = {}
 
 MAX_SIZE = 20
 
@@ -240,9 +243,14 @@ class PeepholeOptimizer:
         if instr.arg > len(self.const_stack):
             return
 
-        next_instr = self.get_next_instr("COMPARE_OP")
-        if next_instr is None or next_instr.arg not in (Compare.IN, Compare.NOT_IN):
-            return
+        if sys.version_info < (3, 9):
+            next_instr = self.get_next_instr("COMPARE_OP")
+            if next_instr is None or next_instr.arg not in (Compare.IN, Compare.NOT_IN):
+                return
+        else:
+            next_instr = self.get_next_instr("CONTAINS_OP")
+            if next_instr is None:
+                return
 
         self.replace_container_of_consts(instr, container_type)
         return True


Test patch:
diff --git a/tests/test_instr.py b/tests/test_instr.py
index 991211d..b2f7df1 100644
--- a/tests/test_instr.py
+++ b/tests/test_instr.py
@@ -356,5 +356,27 @@ class InstrTests(TestCase):
         self.assertIs(f()(), mutable_datum)
 
 
+class CompareTests(TestCase):
+    def test_compare_ops(self):
+        from bytecode import Bytecode, Instr
+
+        def f():
+            pass
+
+        params = zip(iter(Compare), (True, True, False, True, False, False))
+        for cmp, expected in params:
+            with self.subTest():
+                f.__code__ = Bytecode(
+                    [
+                        Instr("LOAD_CONST", 24),
+                        Instr("LOAD_CONST", 42),
+                        Instr("COMPARE_OP", cmp),
+                        Instr("RETURN_VALUE"),
+                    ]
+                ).to_code()
+
+                self.assertIs(f(), expected)
+
+
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover
diff --git a/tests/test_peephole_opt.py b/tests/test_peephole_opt.py
index 28cb43a..48f0bab 100644
--- a/tests/test_peephole_opt.py
+++ b/tests/test_peephole_opt.py
@@ -257,6 +257,12 @@ class Tests(TestCase):
 
     def test_build_list(self):
         # test = x in [1, 2, 3]
+        compare_instr = (
+            Instr("COMPARE_OP", Compare.IN)
+            if sys.version_info < (3, 9)
+            else Instr("CONTAINS_OP", 0)
+        )
+
         code = Bytecode(
             [
                 Instr("LOAD_NAME", "x"),
@@ -264,7 +270,7 @@ class Tests(TestCase):
                 Instr("LOAD_CONST", 2),
                 Instr("LOAD_CONST", 3),
                 Instr("BUILD_LIST", 3),
-                Instr("COMPARE_OP", Compare.IN),
+                compare_instr,
                 Instr("STORE_NAME", "test"),
             ]
         )
@@ -273,7 +279,7 @@ class Tests(TestCase):
             code,
             Instr("LOAD_NAME", "x"),
             Instr("LOAD_CONST", (1, 2, 3)),
-            Instr("COMPARE_OP", Compare.IN),
+            compare_instr,
             Instr("STORE_NAME", "test"),
         )
 
@@ -381,6 +387,12 @@ class Tests(TestCase):
 
     def test_build_set(self):
         # test = x in {1, 2, 3}
+        compare_instr = (
+            Instr("COMPARE_OP", Compare.IN)
+            if sys.version_info < (3, 9)
+            else Instr("CONTAINS_OP", 0)
+        )
+
         code = Bytecode(
             [
                 Instr("LOAD_NAME", "x"),
@@ -388,7 +400,7 @@ class Tests(TestCase):
                 Instr("LOAD_CONST", 2),
                 Instr("LOAD_CONST", 3),
                 Instr("BUILD_SET", 3),
-                Instr("COMPARE_OP", Compare.IN),
+                compare_instr,
                 Instr("STORE_NAME", "test"),
             ]
         )
@@ -397,10 +409,11 @@ class Tests(TestCase):
             code,
             Instr("LOAD_NAME", "x"),
             Instr("LOAD_CONST", frozenset((1, 2, 3))),
-            Instr("COMPARE_OP", Compare.IN),
+            compare_instr,
             Instr("STORE_NAME", "test"),
         )
 
+    @unittest.skipIf(sys.version_info >= (3, 9), "enums not available in Python>=3.9")
     def test_compare_op_unary_not(self):
         for op, not_op in (
             (Compare.IN, Compare.NOT_IN),  # in => not in