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

Ensure opcode names and args fit in disassembly output #66548

Closed
ncoghlan opened this issue Sep 7, 2014 · 3 comments
Closed

Ensure opcode names and args fit in disassembly output #66548

ncoghlan opened this issue Sep 7, 2014 · 3 comments
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@ncoghlan
Copy link
Contributor

ncoghlan commented Sep 7, 2014

BPO 22352
Nosy @ncoghlan, @serhiy-storchaka
PRs
  • bpo-22352: Adjust widths in the output of dis.dis() for large line… #1153
  • 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 = None
    closed_at = <Date 2017-04-19.17:43:19.203>
    created_at = <Date 2014-09-07.04:58:11.556>
    labels = ['3.7', 'type-feature', 'library']
    title = 'Ensure opcode names and args fit in disassembly output'
    updated_at = <Date 2017-04-19.17:43:19.203>
    user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2017-04-19.17:43:19.203>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-04-19.17:43:19.203>
    closer = 'serhiy.storchaka'
    components = ['Library (Lib)']
    creation = <Date 2014-09-07.04:58:11.556>
    creator = 'ncoghlan'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 22352
    keywords = []
    message_count = 3.0
    messages = ['226530', '291710', '291889']
    nosy_count = 2.0
    nosy_names = ['ncoghlan', 'serhiy.storchaka']
    pr_nums = ['1153']
    priority = 'low'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue22352'
    versions = ['Python 3.7']

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Sep 7, 2014

    While exploring display options for bpo-11822, I found that the new matrix multiplication opcode names (BINARY_MATRIX_MULTIPLY and INPLACE_MATRIX_MULTIPLY) don't fit in the nominal field width in the disassembly output (which is currently 20 characters).

    These two clock in at 22 and 23 characters respectively.

    In practice, they do fit, since neither takes on argument, which effectively allows an extra 5 characters (while still looking neat) and unlimited characters if we ignore expanding past the column of opcode arguments.

    However, it would be good to:

    1. Factor out the opname and oparg sizes to private class attributes on dis.Instruction

    2. have a test in test_dis that scans dis.opnames and ensures all opcodes < dis.HAVE_ARGUMENT have names shorter than the combined length of the two fields, and that all opcodes >= HAVE_ARGUMENT will fit in the opname field, even with an argument present.

    Have such a test will ensure any new opcodes added can be displayed without any problems, rather than anyone having to remember to check manually.

    @ncoghlan ncoghlan self-assigned this Sep 7, 2014
    @ncoghlan ncoghlan added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir labels Sep 7, 2014
    @ncoghlan ncoghlan removed their assignment Jun 28, 2015
    @serhiy-storchaka
    Copy link
    Member

    Proposed patch adjusts column widths of the output of dis.dis() for large line numbers and instruction offsets and adds tests for widths of opcode names.

    Unfortunately this test is failed for two new opcodes: BUILD_MAP_UNPACK_WITH_CALL and BUILD_TUPLE_UNPACK_WITH_CALL (26 and 28 characters respectively).

    @serhiy-storchaka serhiy-storchaka added 3.7 (EOL) end of life type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Apr 15, 2017
    @serhiy-storchaka
    Copy link
    Member

    New changeset d90045f by Serhiy Storchaka in branch 'master':
    bpo-22352: Adjust widths in the output of dis.dis() for large line numbers and (bpo-1153)
    d90045f

    @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
    3.7 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants