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

instruction offsets.

Add tests for widths of opcode names.

3 files changed, 77 insertions, 8 deletions

diff --git a/Lib/dis.py b/Lib/dis.py
index f93d5b22d4..f3c18a5fde 100644
--- a/Lib/dis.py
+++ b/Lib/dis.py
@@ -175,6 +175,9 @@ _Instruction.offset.__doc__ = "Start index of operation within bytecode sequence
 _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
 
@@ -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
@@ -214,12 +218,12 @@ class Instruction(_Instruction):
         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 + ')')
@@ -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):
@@ -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."""
diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py
index 980ae16c65..e614b718ee 100644
--- a/Lib/test/test_dis.py
+++ b/Lib/test/test_dis.py
@@ -175,6 +175,13 @@ _BIG_LINENO_FORMAT = """\
               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)
@@ -360,6 +367,17 @@ class DisTests(unittest.TestCase):
         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'):
+                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)
 
@@ -387,13 +405,45 @@ class DisTests(unittest.TestCase):
             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)
diff --git a/Misc/NEWS b/Misc/NEWS
index 4dc7b088b7..e57faacf61 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -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(),