FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

GH-103963: Make dis display names of args for intrinsics opcodes by jkchandalia · Pull Request #104029 · python/cpython · GitHub

/ cpython Public

GH-103963: Make dis display names of args for intrinsics opcodes - #104029

Merged
JelleZijlstra merged 15 commits into
python:mainfrom
jkchandalia:display_intrinsics_names
May 3, 2023
ConversationCommits (15)ChecksFiles changed
Merged

GH-103963: Make dis display names of args for intrinsics opcodes#104029
JelleZijlstra merged 15 commits into
python:mainfrom
jkchandalia:display_intrinsics_names

Conversation

jkchandalia commented May 1, 2023 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

Make the dis module display the names of the args for opcodes CALL_INTRINSIC_*.

Fixes #103963

bedevere-bot mentioned this pull request May 1, 2023
Closed

jkchandalia commented May 1, 2023

Copy link
Copy Markdown
Contributor Author

FYI @iritkatriel @nanjekyejoannah @JelleZijlstra

JelleZijlstra reviewed May 1, 2023
View reviewed changes

JelleZijlstra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good, a few comments. You'll also have to add a NEWS entry.

Comment thread Lib/test/test_dis.py Outdated
Comment thread Tools/build/generate_opcode_h.py Outdated
iritkatriel reviewed May 1, 2023
View reviewed changes
Comment thread Tools/build/generate_opcode_h.py Outdated

jkchandalia commented May 2, 2023

Copy link
Copy Markdown
Contributor Author

@iritkatriel @JelleZijlstra I've reviewed your comments and updated the PR. Please let me know if you see anything else or if I should convert to normal PR.

JelleZijlstra reviewed May 2, 2023
View reviewed changes
Comment thread Lib/test/test_dis.py Outdated
JelleZijlstra approved these changes May 2, 2023
View reviewed changes

JelleZijlstra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks great!

JelleZijlstra commented May 2, 2023

Copy link
Copy Markdown
Member

I'll give Irit some time to take another look, but I think this is ready to merge.

hugovk mentioned this pull request May 2, 2023
Closed
iritkatriel reviewed May 2, 2023
View reviewed changes
Comment thread Include/internal/pycore_intrinsics.h Outdated


/* Binary Functions: */
#define INTRINSIC_INVALID 0

iritkatriel May 2, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks like INTRINSIC_INVALID is defined twice?

jkchandalia May 2, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Updated to differentiate between invalid for the two CALL_INTRINSIC_* opcodes.

iritkatriel marked this pull request as ready for review May 2, 2023 17:46
JelleZijlstra merged commit 872cbc6 into python:main May 3, 2023
erlend-aasland mentioned this pull request May 3, 2023
Merged
carljm mentioned this pull request May 4, 2023
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

JelleZijlstra JelleZijlstra approved these changes
iritkatriel iritkatriel approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

dis: Show names of intrinsics

4 participants

Back | FazBrowse Home | New Git URL