GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-103963: Make dis display names of args for intrinsics opcodes by jkchandalia · Pull Request #104029 · python/cpython

Conversation

@jkchandalia

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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

Fixes #103963

  • Issue: dis: Show names of intrinsics #103963

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
jkchandalia added 10 commits April 29, 2023 19:05
@jkchandalia
Add intrinsic names to opcode.py
119c628
@jkchandalia
Use intrinsics names define in opcode.py in dis.py
74ddc3e
@jkchandalia
Add single test for 3 CALL_INTRINSIC_1 codes and update one existing …
0ecf7f5 
…test
@jkchandalia
Update opcode.h generation to include intrinsics codes
9d93d89
@jkchandalia
Update opcode.h to include the new intrinsics sections
3056de5
@jkchandalia
Revert changes to opcode.h for intrinsics
be813ee
@jkchandalia
Add test for CALL_INTRINSIC_2
0f10791
@jkchandalia
Update generate_opcode to generate intrinsics header file
89bfd12
@jkchandalia
Update intrinsics outfile
e10dae1
@jkchandalia
Regenerate intrinsics header file
f78e70e
@bedevere-bot bedevere-bot added the awaiting review label May 1, 2023
@bedevere-bot bedevere-bot mentioned this pull request May 1, 2023
dis: Show names of intrinsics #103963
Closed
@jkchandalia

jkchandalia commented May 1, 2023

Copy link
Copy Markdown
Contributor Author

FYI @iritkatriel @nanjekyejoannah @JelleZijlstra
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
JelleZijlstra
JelleZijlstra reviewed May 1, 2023
View reviewed changes

@JelleZijlstra JelleZijlstra left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_dis.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Tools/build/generate_opcode_h.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel
iritkatriel reviewed May 1, 2023
View reviewed changes
Comment thread Tools/build/generate_opcode_h.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
blurb-it Bot and others added 3 commits May 1, 2023 17:58
@blurb-it
📜🤖 Added by blurb_it.
2267933
@jkchandalia
Move static statements to footer and add newline
db9a1e9
@jkchandalia
Test CALL_INTRINSIC_2 arg name with less hardcoded bytecode
f034688
@jkchandalia

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
JelleZijlstra
JelleZijlstra reviewed May 2, 2023
View reviewed changes
Comment thread Lib/test/test_dis.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@jkchandalia
Use assertIn
cca18e9
JelleZijlstra
JelleZijlstra approved these changes May 2, 2023
View reviewed changes

@JelleZijlstra JelleZijlstra left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks great!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels May 2, 2023
@JelleZijlstra

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk hugovk mentioned this pull request May 2, 2023
Docs: Fix build preview banner #104082
Closed
iritkatriel
iritkatriel reviewed May 2, 2023
View reviewed changes
Comment thread Include/internal/pycore_intrinsics.h Outdated


/* Binary Functions: */
#define INTRINSIC_INVALID 0

@iritkatriel iritkatriel May 2, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks like INTRINSIC_INVALID is defined twice?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@jkchandalia jkchandalia May 2, 2023

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@iritkatriel
iritkatriel marked this pull request as ready for review May 2, 2023 17:46
@jkchandalia
Differentiate between intrinsic invalid args
abd3ff1
iritkatriel
iritkatriel approved these changes May 2, 2023
View reviewed changes
@JelleZijlstra
JelleZijlstra merged commit 872cbc6 into python:main May 3, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label May 3, 2023
@erlend-aasland erlend-aasland mentioned this pull request May 3, 2023
gh-104146: Remove unused 'second_pass_replacements' from clinic.py #104147
Merged
@carljm carljm mentioned this pull request May 4, 2023
gh-103963: fix 'make regen-opcode' in out-of-tree builds #104177
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes
@iritkatriel iritkatriel iritkatriel approved these changes

Uh oh!

There was an error while loading. Please reload this page.

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

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@jkchandalia @JelleZijlstra @iritkatriel @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.