8000 bpo-22352: Adjust widths in the output of dis.dis() for large line… by serhiy-storchaka · Pull Request #1153 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-22352: Adjust widths in the output of dis.dis() for large line… #1153

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 19, 2017
Merged

bpo-22352: Adjust widths in the output of dis.dis() for large line… #1153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8af2a3b
bpo-22352: Adjust widths in the output of dis.dis() for large line nu…
serhiy-storchaka Apr 15, 2017
042712d
Merge branch 'master' into dis-adjust-width
serhiy-storchaka Apr 19, 2017
c1a3c15
Merge branch 'master' into dis-adjust-width
serhiy-storchaka Apr 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions Lib/dis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@ def show_code(co, *, file=None):
_Instruction.starts_line.__doc__ = "Line started by this opcode (if any), otherwise None"
_Instruction.is_jump_target.__doc__ = "True if other code jumps to here, otherwise False"

_OPNAME_WIDTH = 20
_OPARG_WIDTH = 5

class Instruction(_Instruction):
"""Details for a bytecode operation

Expand All @@ -189,11 +192,12 @@ class Instruction(_Instruction):
is_jump_target - True if other code jumps to here, otherwise False
"""

def _disassemble(self, lineno_width=3, mark_as_current=False):
def _disassemble(self, lineno_width=3, mark_as_current=False, offset_width=4):
"""Format instruction details for inclusion in disassembly output

*lineno_width* sets the width of the line number field (0 omits it)
*mark_as_current* inserts a '-->' marker arrow as part of the line
*offset_width* sets the width of the instruction offset field
"""
fields = []
# Column: Source code line number
Expand All @@ -214,12 +218,12 @@ def _disassemble(self, lineno_width=3, mark_as_current=False):
else:
fields.append(' ')
# Column: Instruction offset from start of code sequence
fields.append(repr(self.offset).rjust(4))
fields.append(repr(self.offset).rjust(offset_width))
# Column: Opcode name
fields.append(self.opname.ljust(20))
fields.append(self.opname.ljust(_OPNAME_WIDTH))
# Column: Opcode argument
if self.arg is not None:
fields.append(repr(self.arg).rjust(5))
fields.append(repr(self.arg).rjust(_OPARG_WIDTH))
# Column: Opcode argument details
if self.argrepr:
fields.append('(' + self.argrepr + ')')
Expand Down Expand Up @@ -339,8 +343,19 @@ def _disassemble_bytes(code, lasti=-1, varnames=None, names=None,
*, file=None, line_offset=0):
# Omit the line number column entirely if we have no line number info
show_lineno = linestarts is not None
# TODO?: Adjust width upwards if max(linestarts.values()) >= 1000?
lineno_width = 3 if show_lineno else 0
if show_lineno:
maxlineno = max(linestarts.values()) + line_offset
if maxlineno >= 1000:
lineno_width = len(str(maxlineno))
else:
lineno_width = 3
else:
lineno_width = 0
maxoffset = len(code) - 2
if maxoffset >= 10000:
offset_width = len(str(maxoffset))
else:
offset_width = 4
for instr in _get_instructions_bytes(code, varnames, names,
constants, cells, linestarts,
line_offset=line_offset):
Expand All @@ -350,7 +365,8 @@ def _disassemble_bytes(code, lasti=-1, varnames=None, names=None,
if new_source_line:
print(file=file)
is_current_instr = instr.offset == lasti
print(instr._disassemble(lineno_width, is_current_instr), file=file)
print(instr._disassemble(lineno_width, is_current_instr, offset_width),
file=file)

def _disassemble_str(source, *, file=None):
"""Compile the source string, then disassemble the code object."""
Expand Down
52 changes: 51 additions & 1 deletion Lib/test/test_dis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,13 @@ def bug1333982(x=[]):
6 RETURN_VALUE
"""

_BIG_LINENO_FORMAT2 = """\
%4d 0 LOAD_GLOBAL 0 (spam)
2 POP_TOP
4 LOAD_CONST 0 (None)
6 RETURN_VALUE
"""

dis_module_expected_results = """\
Disassembly of f:
4 0 LOAD_CONST 0 (None)
Expand Down Expand Up @@ -360,6 +367,17 @@ def test_boundaries(self):
self.assertEqual(dis.opmap["EXTENDED_ARG"], dis.EXTENDED_ARG)
self.assertEqual(dis.opmap["STORE_NAME"], dis.HAVE_ARGUMENT)

def test_widths(self):
for opcode, opname in enumerate(dis.opname):
if opname in ('BUILD_MAP_UNPACK_WITH_CALL',
'BUILD_TUPLE_UNPACK_WITH_CALL'):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I don't think it should block this change (since it's an existing problem) I wonder if it might it be worth special-casing these for display purposes using something like:

BUILD_MAP_UNPACK_WITH_CALL
...MAP_UNPACK+CALL
12345678901234567890

BUILD_TUPLE_UNPACK_WITH_CALL
...TUPLE_UNPACK+CALL
12345678901234567890

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I don't understand your idea. This already is special-cased, the test is skipped for these names.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant in the actual display code, as I assume the columns are currently misaligned for rows containing these two opcodes, whereas they wouldn't be given the abbreviations (...TUPLE_UNPACK+CALL is 20 characters, while ...MAP_UNPACK+CALL is 18).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to misalign columns than output corrupted opcode names.

It may be worth to rename these two opcodes, but this is different issue.

continue
with self.subTest(opname=opname):
width = dis._OPNAME_WIDTH
if opcode < dis.HAVE_ARGUMENT:
width += 1 + dis._OPARG_WIDTH
self.assertLessEqual(len(opname), width)

def test_dis(self):
self.do_disassembly_test(_f, dis_f)

Expand Down Expand Up @@ -387,13 +405,45 @@ def func(count):
self.do_disassembly_test(func(i), expected)

# Test some larger ranges too
for i in range(300, 5000, 10):
for i in range(300, 1000, 10):
expected = _BIG_LINENO_FORMAT % (i + 2)
self.do_disassembly_test(func(i), expected)

for i in range(1000, 5000, 10):
expected = _BIG_LINENO_FORMAT2 % (i + 2)
self.do_disassembly_test(func(i), expected)

from test import dis_module
self.do_disassembly_test(dis_module, dis_module_expected_results)

def test_big_offsets(self):
def func(count):
namespace = {}
func = "def foo(x):\n " + ";".join(["x = x + 1"] * count) + "\n return x"
exec(func, namespace)
return namespace['foo']

def expected(count, w):
s = ['''\
%*d LOAD_FAST 0 (x)
%*d LOAD_CONST 1 (1)
%*d BINARY_ADD
%*d STORE_FAST 0 (x)
''' % (w, 8*i, w, 8*i + 2, w, 8*i + 4, w, 8*i + 6)
for i in range(count)]
s += ['''\

3 %*d LOAD_FAST 0 (x)
%*d RETURN_VALUE
''' % (w, 8*count, w, 8*count + 2)]
s[0] = ' 2' + s[0][3:]
return ''.join(s)

for i in range(1, 5):
self.do_disassembly_test(func(i), expected(i, 4))
self.do_disassembly_test(func(1249), expected(1249, 4))
self.do_disassembly_test(func(1250), expected(1250, 5))

def test_disassemble_str(self):
self.do_disassembly_test(expr_str, dis_expr_str)
self.do_disassembly_test(simple_stmt_str, dis_simple_stmt_str)
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,9 @@ Extension Modules
Library
-------

- bpo-22352: Column widths in the output of dis.dis() are now adjusted for
large line numbers and instruction offsets.

- bpo-30061: Fixed crashes in IOBase methods __next__() and readlines() when
readline() or __next__() respectively return non-sizeable object.
Fixed possible other errors caused by not checking results of PyObject_Size(),
Expand Down
0