Skip to content

Commit

Permalink
Fix a bug in generate_opcode_diffs and add a missing 3.10 opcode.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rchen152 committed Feb 18, 2022
1 parent fdc0c37 commit 0e7f41b
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 21 deletions.
46 changes: 35 additions & 11 deletions pytype/pyc/generate_opcode_diffs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 "<i>" 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,
Expand Down Expand Up @@ -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 = []
Expand Down Expand Up @@ -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__':
Expand Down
26 changes: 20 additions & 6 deletions pytype/pyc/generate_opcode_diffs_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -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__ = ()
Expand All @@ -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):
Expand All @@ -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()
11 changes: 7 additions & 4 deletions pytype/pyc/opcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -754,10 +754,6 @@ class POP_FINALLY(OpcodeWithArg):
__slots__ = ()


class RERAISE(Opcode):
__slots__ = ()


class WITH_EXCEPT_START(Opcode):
__slots__ = ()

Expand Down Expand Up @@ -830,6 +826,11 @@ class ROT_N(OpcodeWithArg):
__slots__ = ()


class RERAISE(OpcodeWithArg):
FLAGS = HAS_ARGUMENT
__slots__ = ()


class GEN_START(OpcodeWithArg):
FLAGS = HAS_ARGUMENT
__slots__ = ()
Expand Down Expand Up @@ -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,
})
Expand Down

0 comments on commit 0e7f41b

Please sign in to comment.