Skip to content
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.

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

Provide dis.Bytecode based equivalent of dis.distb #62116

Closed
ncoghlan opened this issue May 6, 2013 · 10 comments
Closed

Provide dis.Bytecode based equivalent of dis.distb #62116

ncoghlan opened this issue May 6, 2013 · 10 comments
Assignees
Labels
type-feature A feature request or enhancement

Comments

@ncoghlan
Copy link
Contributor

ncoghlan commented May 6, 2013

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
    Copy link
    Contributor Author

    ncoghlan commented May 6, 2013

    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.

    @ncoghlan ncoghlan added the type-feature A feature request or enhancement label May 6, 2013
    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Aug 21, 2013

    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
    Copy link
    Mannequin

    PCManticore mannequin commented Oct 24, 2013

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

    @ncoghlan
    Copy link
    Contributor Author

    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
    Copy link
    Mannequin

    PCManticore mannequin commented Oct 24, 2013

    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
    Copy link
    Mannequin

    PCManticore mannequin commented Nov 21, 2013

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

    @ncoghlan
    Copy link
    Contributor Author

    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
    Copy link
    Mannequin

    PCManticore mannequin commented Nov 21, 2013

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

    @ncoghlan ncoghlan self-assigned this Nov 22, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 22, 2013

    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

    @python-dev python-dev mannequin closed this as completed Nov 22, 2013
    @ncoghlan
    Copy link
    Contributor Author

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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant