From 0e7f41b113aa590d0a560776451e47eb4c661530 Mon Sep 17 00:00:00 2001 From: rechen Date: Thu, 17 Feb 2022 19:57:34 -0800 Subject: [PATCH] Fix a bug in generate_opcode_diffs and add a missing 3.10 opcode. The script missed an opcode (RERAISE) whose index changed in 3.10. Technically speaking, we should be using different class definitions for RERAISE in pyc/opcodes.py depending on the target version, but using the 3.10 definition in older versions doesn't seem to cause any issues, so it's simpler to just always use the newest definition. For #1022. PiperOrigin-RevId: 429463279 --- pytype/pyc/generate_opcode_diffs.py | 46 ++++++++++++++++++------ pytype/pyc/generate_opcode_diffs_test.py | 26 ++++++++++---- pytype/pyc/opcodes.py | 11 +++--- 3 files changed, 62 insertions(+), 21 deletions(-) diff --git a/pytype/pyc/generate_opcode_diffs.py b/pytype/pyc/generate_opcode_diffs.py index 8e09ae1c4..5d01fe5b9 100644 --- a/pytype/pyc/generate_opcode_diffs.py +++ b/pytype/pyc/generate_opcode_diffs.py @@ -20,6 +20,10 @@ obvious where it goes. Then add the new mapping to the mapping dict in the top-level dis function in the same module. * "OPCODE STUB IMPLEMENTATIONS" are new methods that should be added to vm.py. +If the output contains "NOTE:" lines about modified opcodes, then there are +opcode names that exist in both versions but at different indices. For such +opcodes, the script generates a new class definition for pyc/opcodes.py but does +not generate a new stub implementation for vm.py. """ import json @@ -36,12 +40,16 @@ def generate_diffs(argv): # Create a temporary script to print out information about the opcode mapping # of the Python version that the script is running under, and use subprocess # to run the script under the two versions we want to diff. + # opmap is a mapping from opcode name to index, e.g. {'RERAISE': 119}. + # opname is a sequence of opcode names in which the sequence index corresponds + # to the opcode index. A name of "" means that i is an unused index. with tempfile.NamedTemporaryFile(mode='w', suffix='.py') as f: f.write(textwrap.dedent(""" import dis import json output = { 'opmap': dis.opmap, + 'opname': dis.opname, 'HAVE_ARGUMENT': dis.HAVE_ARGUMENT, 'HAS_CONST': dis.hasconst, 'HAS_NAME': dis.hasname, @@ -69,17 +77,25 @@ def generate_diffs(argv): # index: ('DELETE', deleted opcode) # index: ('CHANGE', old opcode at this index, new opcode at this index) # index: ('ADD', new opcode) + num_opcodes = len(dis1['opname']) + assert num_opcodes == len(dis2['opname']) changed = {} - for name, op in dis1['opmap'].items(): - if name not in dis2['opmap']: - changed[op] = ('DELETE', name) - - for name, op in dis2['opmap'].items(): - if name not in dis1['opmap']: - if op in changed: - changed[op] = ('CHANGE', changed[op][1], name) + moved = set() + + def is_unset(opname_entry): + return opname_entry == f'<{i}>' + + for i in range(num_opcodes): + if dis1['opname'][i] != dis2['opname'][i]: + if is_unset(dis2['opname'][i]): + changed[i] = ('DELETE', dis1['opname'][i]) else: - changed[op] = ('ADD', name) + if dis2['opname'][i] in dis1['opmap']: + moved.add(dis2['opname'][i]) + if is_unset(dis1['opname'][i]): + changed[i] = ('ADD', dis2['opname'][i]) + else: + changed[i] = ('CHANGE', dis1['opname'][i], dis2['opname'][i]) # Generate opcode classes. classes = [] @@ -124,21 +140,29 @@ def generate_diffs(argv): if diff[0] == 'DELETE': continue name = diff[-1] + if name in moved: + continue stubs.append(['def byte_{}(self, state, op):'.format(name), ' del op', ' return state']) - return classes, diffs, stubs + return classes, diffs, stubs, sorted(moved) def main(argv): - classes, diff, stubs = generate_diffs(argv) + classes, diff, stubs, moved = generate_diffs(argv) print('---- NEW OPCODES (pyc/opcodes.py) ----\n') print('\n\n\n'.join('\n'.join(cls) for cls in classes)) + if moved: + print('\nNOTE: Delete the old class definitions for the following ' + 'modified opcodes: ' + ', '.join(moved)) print('\n---- OPCODE MAPPING DIFF (pyc/opcodes.py) ----\n') print(' ' + '\n '.join(diff)) print('\n---- OPCODE STUB IMPLEMENTATIONS (vm.py) ----\n') print('\n\n'.join(' ' + '\n '.join(stub) for stub in stubs)) + if moved: + print('\nNOTE: The implementations of the following modified opcodes may ' + 'need to be updated: ' + ', '.join(moved)) if __name__ == '__main__': diff --git a/pytype/pyc/generate_opcode_diffs_test.py b/pytype/pyc/generate_opcode_diffs_test.py index ae26f8bf7..9184cfd26 100644 --- a/pytype/pyc/generate_opcode_diffs_test.py +++ b/pytype/pyc/generate_opcode_diffs_test.py @@ -14,13 +14,17 @@ class GenerateOpcodeDiffsTest(unittest.TestCase): def _generate_diffs(self): with mock.patch.object(subprocess, 'run') as mock_run: mapping_38 = json.dumps({ - 'opmap': {'DO_THIS': 1, 'DO_EIGHT': 5}, + 'opmap': {'DO_THIS': 1, 'I_MOVE': 2, 'DO_EIGHT': 5}, + 'opname': ['<0>', 'DO_THIS', 'I_MOVE', '<3>', '<4>', 'DO_EIGHT', + '<6>', '<7>'], 'HAVE_ARGUMENT': 3, 'HAS_CONST': [], 'HAS_NAME': [], }) mapping_39 = json.dumps({ - 'opmap': {'DO_THAT': 4, 'DO_THAT_TOO': 5, 'DO_NINE': 7}, + 'opmap': {'I_MOVE': 3, 'DO_THAT': 4, 'DO_THAT_TOO': 5, 'DO_NINE': 7}, + 'opname': ['<0>', '<1>', '<2>', 'I_MOVE', 'DO_THAT', 'DO_THAT_TOO', + '<6>', 'DO_NINE'], 'HAVE_ARGUMENT': 6, 'HAS_CONST': [7], 'HAS_NAME': [5, 7], @@ -30,8 +34,12 @@ def _generate_diffs(self): return generate_opcode_diffs.generate_diffs(['3.8', '3.9']) def test_classes(self): - classes, _, _ = self._generate_diffs() - do_that, do_that_too, do_nine = classes + classes, _, _, _ = self._generate_diffs() + i_move, do_that, do_that_too, do_nine = classes + self.assertMultiLineEqual('\n'.join(i_move), textwrap.dedent(""" + class I_MOVE(Opcode): + __slots__ = () + """).strip()) self.assertMultiLineEqual('\n'.join(do_that), textwrap.dedent(""" class DO_THAT(Opcode): __slots__ = () @@ -48,16 +56,18 @@ class DO_NINE(OpcodeWithArg): """).strip()) def test_diff(self): - _, diff, _ = self._generate_diffs() + _, diff, _, _ = self._generate_diffs() self.assertMultiLineEqual('\n'.join(diff), textwrap.dedent(""" 1: None, # was DO_THIS in 3.8 + 2: None, # was I_MOVE in 3.8 + 3: I_MOVE, 4: DO_THAT, 5: DO_THAT_TOO, # was DO_EIGHT in 3.8 7: DO_NINE, """).strip()) def test_stubs(self): - _, _, stubs = self._generate_diffs() + _, _, stubs, _ = self._generate_diffs() do_that, do_that_too, do_nine = stubs self.assertMultiLineEqual('\n'.join(do_that), textwrap.dedent(""" def byte_DO_THAT(self, state, op): @@ -75,6 +85,10 @@ def byte_DO_NINE(self, state, op): return state """).strip()) + def test_moved(self): + _, _, _, moved = self._generate_diffs() + self.assertEqual(moved, ['I_MOVE']) + if __name__ == '__main__': unittest.main() diff --git a/pytype/pyc/opcodes.py b/pytype/pyc/opcodes.py index b8ceabef6..ab26737a6 100644 --- a/pytype/pyc/opcodes.py +++ b/pytype/pyc/opcodes.py @@ -754,10 +754,6 @@ class POP_FINALLY(OpcodeWithArg): __slots__ = () -class RERAISE(Opcode): - __slots__ = () - - class WITH_EXCEPT_START(Opcode): __slots__ = () @@ -830,6 +826,11 @@ class ROT_N(OpcodeWithArg): __slots__ = () +class RERAISE(OpcodeWithArg): + FLAGS = HAS_ARGUMENT + __slots__ = () + + class GEN_START(OpcodeWithArg): FLAGS = HAS_ARGUMENT __slots__ = () @@ -1010,7 +1011,9 @@ def _overlay_mapping(mapping, new_entries): 32: MATCH_SEQUENCE, 33: MATCH_KEYS, 34: COPY_DICT_WITHOUT_KEYS, + 48: None, # was RERAISE in 3.9 99: ROT_N, + 119: RERAISE, 129: GEN_START, 152: MATCH_CLASS, })