Skip to content

gh-99944: remove hardcoded check that op is LOAD_CONST #99943

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

Closed
wants to merge 1 commit into from

Conversation

chilaxan
Copy link
Contributor

@chilaxan chilaxan commented Dec 2, 2022

3.11 adds a new opcode KW_NAMES that has an entry in co_consts, however the hardcoded check here causes dis to not grab the constant

3.11 adds a new opcode KW_NAMES that has an entry in co_consts, however the hardcoded check here causes dis to not grab the constant
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Please, add a test-case and change your PR title to start with gh-99944:

@chilaxan chilaxan changed the title remove hardcoded check that op is LOAD_CONST gh-99944: remove hardcoded check that op is LOAD_CONST Dec 4, 2022
@chilaxan
Copy link
Contributor Author

chilaxan commented Dec 4, 2022

Please, add a test-case and change your PR title to start with gh-99944:

can you give me a resource for adding a test case?

Copy link
Member

@carljm carljm left a comment

Choose a reason for hiding this comment

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

can you give me a resource for adding a test case?

The tests for Lib/dis.py are in Lib/test/test_dis.py. You should be able to look through existing test cases there and find some tests that are suitable to copy and modify. You just want a test that verifies the dis output for some code that includes KW_NAMES opcode.

Also, please add a What's New entry.

If those things are done, this PR looks good to me.

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@AlexWaygood
Copy link
Member

Just to build on @carljm's excellent advice — there's also some resources on running and writing tests for CPython here: https://devguide.python.org/testing/run-write-tests/

@iritkatriel
Copy link
Member

This was completed during the sprint in #103856.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants