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

Clean up Python 3.4 API additions in the dis module #63577

Closed
ncoghlan opened this issue Oct 24, 2013 · 6 comments
Closed

Clean up Python 3.4 API additions in the dis module #63577

ncoghlan opened this issue Oct 24, 2013 · 6 comments
Assignees

Comments

@ncoghlan
Copy link
Contributor

BPO 19378
Nosy @rhettinger, @ncoghlan, @larryhastings
Files
  • issue19378_dis_module_fixes.diff: Assorted fixes for the dis module
  • 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-06.12:08:50.829>
    created_at = <Date 2013-10-24.13:44:36.616>
    labels = ['release-blocker']
    title = 'Clean up Python 3.4 API additions in the dis module'
    updated_at = <Date 2013-11-06.12:08:50.822>
    user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2013-11-06.12:08:50.822>
    actor = 'python-dev'
    assignee = 'ncoghlan'
    closed = True
    closed_date = <Date 2013-11-06.12:08:50.829>
    closer = 'python-dev'
    components = []
    creation = <Date 2013-10-24.13:44:36.616>
    creator = 'ncoghlan'
    dependencies = []
    files = ['32479']
    hgrepos = []
    issue_num = 19378
    keywords = ['patch']
    message_count = 6.0
    messages = ['201131', '202028', '202053', '202076', '202077', '202261']
    nosy_count = 5.0
    nosy_names = ['rhettinger', 'ncoghlan', 'larry', 'rfk', 'python-dev']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue19378'
    versions = ['Python 3.4']

    @ncoghlan
    Copy link
    Contributor Author

    The "line_offset" parameter in dis.get_instructions is the line number of the first line in the source file: http://docs.python.org/dev/library/dis#dis.get_instructions

    Calling this an offset is a little confusing, since "offset" in the dis docs almost always refers to an instruction offset. bpo-17916 is likely to make this worse, since that will probably involve new "last_offset" and "current_offset" parameters to other APIs.

    Renaming the parameter to "first_line" (since it sets the line number reported for the first line in the code object when iterating) should help make this less confusing.

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Nov 3, 2013

    While working on this, I noticed a number of other issues with the dis API additions for Python 3.4 from bpo-11816. Specifically:

    • the file output redirection API didn't work in some cases that resulted in calls to other disassembly functions
    • Bytecode.show_info() replicates a bad module level API that shouldn't be perpetuated
    • Bytecode.display_code() is better converted to a dis() method that returns a string rather than writing directly to an output stream

    @ncoghlan ncoghlan changed the title Rename "line_offset" parameter in dis.get_instructions to "first_line" Clean up Python 3.4 API additions in the dis module Nov 3, 2013
    @larryhastings
    Copy link
    Contributor

    Would this be a good time for me to ask about publishing the stack effect info? I had to write my own parallel implementation of it for my assembler, so I found it irritating that Python doesn't provide it.

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Nov 4, 2013

    I think it's a good idea in principle, but unrelated to this patch :)

    This one started as reworking line_offset as a more intuitive "first_line" parameter, and then testing and documenting that change proceeded to reveal a number of other issues with the 3.4 API additions :P

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Nov 4, 2013

    There was another change implemented as part of this: trailing whitespace is now stripped from the lines emitted by the disassembler, which made it possible to simplify the tests a bit (since they no longer have to strip that whitespace themselves, they can just do normal string comparisons)

    I rediscovered this existing issue, since the new more comprehensive tests for correct handling of the file parameter didn't have the extra operations to strip the trailing whitespace from each line. Rather than adding it, I just fixed the dissassembler to avoid emitting it in the first place (that was a lot easier now that Instruction._disassemble was the sole place responsible for emitting each line)

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

    python-dev mannequin commented Nov 6, 2013

    New changeset ce8dd299cdc4 by Nick Coghlan in branch 'default':
    Close bpo-19378: address flaws in the new dis module APIs
    http://hg.python.org/cpython/rev/ce8dd299cdc4

    @python-dev python-dev mannequin closed this as completed Nov 6, 2013
    @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
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants