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

gh-102676: Add more convenience properties to dis.Instruction #103969

Merged
merged 13 commits into from Jun 11, 2023

Conversation

tomasr8
Copy link
Contributor

@tomasr8 tomasr8 commented Apr 28, 2023

Closes #102676

Adds start_offset, cache_offset, end_offset, baseopcode,
baseopname, jump_target and oparg to dis.Instruction.

As suggested, the disassembly output is also improved by allowing the opname
to overflow into oparg's space which reduces the number of misaligned opargs.

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Apr 28, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

Adds start_offset, cache_offset, end_offset, baseopcode,
baseopname, jump_target and oparg to dis.Instruction.

Also slightly improves the disassembly output by allowing
opnames to overflow into the space reserved for opargs.
@gvanrossum gvanrossum self-requested a review May 11, 2023 22:56
@gvanrossum gvanrossum self-assigned this May 11, 2023
@gvanrossum
Copy link
Member

Thanks, I will look at this!

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fantastic! I would just like to tweak some of the doc text, and I am wondering if the "excess opname" logic can be simplified.

Doc/library/dis.rst Outdated Show resolved Hide resolved
Doc/library/dis.rst Outdated Show resolved Hide resolved
Doc/library/dis.rst Outdated Show resolved Hide resolved
Lib/dis.py Outdated Show resolved Hide resolved
Lib/dis.py Outdated Show resolved Hide resolved
Lib/dis.py Outdated Show resolved Hide resolved
Lib/dis.py Outdated Show resolved Hide resolved
Lib/dis.py Outdated Show resolved Hide resolved
Lib/dis.py Outdated Show resolved Hide resolved
tomasr8 and others added 9 commits May 25, 2023 20:37
Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>
Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>
Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>
@tomasr8
Copy link
Contributor Author

tomasr8 commented May 25, 2023

Thanks for the detailed review! I fixed the issues with the docs/docstrings and simplified the formatting logic.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! I will land this now.

@gvanrossum gvanrossum merged commit 18d16e9 into python:main Jun 11, 2023
23 checks passed
@tomasr8 tomasr8 deleted the dis-instruction branch June 11, 2023 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make dis.Instruction more useful
3 participants