Provide dis.Bytecode based equivalent of dis.distb

[ncoghlan]

BPO	17916
Nosy	@ncoghlan, @PCManticore
Dependencies	bpo-11816: Refactor the dis module to provide better building blocks for bytecode analysis bpo-19378: Clean up Python 3.4 API additions in the dis module
Files	dis.patch dis_tb.patch dis_tb_3.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/ncoghlan'
closed_at = <Date 2013-11-22.14:57:23.524>
created_at = <Date 2013-05-06.12:54:24.115>
labels = ['type-feature']
title = 'Provide dis.Bytecode based equivalent of dis.distb'
updated_at = <Date 2013-11-22.14:58:24.766>
user = 'https://github.com/ncoghlan'

bugs.python.org fields:

activity = <Date 2013-11-22.14:58:24.766>
actor = 'ncoghlan'
assignee = 'ncoghlan'
closed = True
closed_date = <Date 2013-11-22.14:57:23.524>
closer = 'python-dev'
components = []
creation = <Date 2013-05-06.12:54:24.115>
creator = 'ncoghlan'
dependencies = ['11816', '19378']
files = ['31399', '32333', '32762']
hgrepos = []
issue_num = 17916
keywords = ['patch']
message_count = 10.0
messages = ['188528', '195776', '201124', '201133', '201160', '203619', '203623', '203671', '203775', '203776']
nosy_count = 3.0
nosy_names = ['ncoghlan', 'Claudiu.Popa', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue17916'
versions = ['Python 3.4']

[ncoghlan]

bpo-11816 adds a new dis.Bytecode API that replaces most of the dis functions, but doesn't cover dis.distb.

A dis.Bytecode.from_tb class method, or a TracebackBytecode subclass to handle that use case may be desirable.

[PCManticore]

Here's a basic patch, using a classmethod. It doesn't support the full distb API (if the traceback is not given, try to retrieve the last one), because Bytecode.from_tb() looks pretty weird.

[PCManticore]

Nick, could you review this, please? It would be nice if we can flesh it out and add it to 3.4.

[ncoghlan]

It's not quite that simple - the instruction responsible for the exception also needs to be recorded, and Bytecode doesn't currently allow for that (look at the way the "lasti" arg to disassemble is used in disttb, and then how that gets passed to the "mark_as_current" flag in Instruction._disassemble by _disassemble_bytes).

So, on reflection, I think a TracebackBytecode class may be a better idea, exposing "bytecode" and "last_offset" attributes.

Rather than iteration just producing instructions, it would produce (instruction, is_current) 2-tuples, and display_code would call through to "self.bytecode.display_code(self.last_offset)" (there would be no show_info() or info() methods on TracebackBytecode).

Bytecode.display_code would gain a new "current_offset" parameter, which it would pass through to _disassemble_bytes as the "lasti" parameter.

Reviewing this also made me realise "line_offset" in dis.get_instructions is misnamed - see bpo-19378.

[PCManticore]

Here's the updated version according to your comments. Because there is no need for show_info() and info(), the current class is not a subclass of Bytecode (as I implied from your first message). After bpo-19378 is fixed, we could change the line_offset with something else in TracebackBytecode.__iter__.

[PCManticore]

Nick, what's your opinion on my latest patch?

[ncoghlan]

My apologies for not reviewing this earlier, it's been a somewhat hectic month. Thanks for pinging the ticket while there's still a chance to get this into 3.4 :)

After my last round of updates to dis.Bytecode removed the potential for confusion between line_offset and current_offset, I'm back to thinking it makes more sense to build this feature directly into dis.Bytecode rather than using a separate type.

1. Add the "current_offset" attribute to dis.Bytecode
2. Add a "current_offset" parameter to dis.Bytecode.__init__ (defaulting to None)
3. In dis.Bytecode.dis(), pass "current_offset" as the value for "lasti" in the call to _disassemble_bytes (except for passing -1 if current_offset is None)
4. Add a "from_traceback()" class method that sets current_offset appropriately in the call to build the instance

[PCManticore]

Thanks, Nick! Attached the new version, I hope that I did it right this time.

[python-dev]

New changeset d71251d9fbbe by Nick Coghlan in branch 'default':
Close bpo-17916: dis.Bytecode based replacement for distb
http://hg.python.org/cpython/rev/d71251d9fbbe

[ncoghlan]

Thank you for the patch! It's nice to have this included for the initial general availability of the new disassembly API :)