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

Python 3.13 Support Draft #153

Merged
merged 39 commits into from
Nov 8, 2024
Merged

Python 3.13 Support Draft #153

merged 39 commits into from
Nov 8, 2024

Conversation

2elli
Copy link
Contributor

@2elli 2elli commented Oct 9, 2024

Added basic 3.13 support.

Currently passing all unit tests with make check.
This PR also includes some small fixes to 3.12.

Still work in progress. While my fork passes testing, it still breaks in some major circumstances. For example, I cannot self disassemble xdis/opcodes/opcode_313.py, see TODO.

TODO (not exhaustive)

Formatting

  • Lots of oparg formatting to fix. Ex. some new ops SET_FUNCTION_ATTRIBUTE and CALL_KW need to have formatting fixed.
  • Running into an error with certain bytecode that uses EXTENDED_ARG. In some cases, I am finding EXTENDED_ARG with an oparg of None, leading to type comparison errors and incorrect behavior.

"drop-in" dis support

  • dis.disassemble and dis.distb have a new parameter show_offsets.
  • Instruction class in dis has new fields start_offset, cache_offset, end_offset, baseopname, baseopcode, jump_target, oparg, line_number and cache_info.

@2elli
Copy link
Contributor Author

2elli commented Oct 10, 2024

@rocky Is there a best way to find out what the pop and push values should be for defining a new instruction with def_op? I am running into issues finding the correct pop and push values for the new CALL_KW op.

@rocky
Copy link
Owner

rocky commented Oct 11, 2024

@rocky Is there a best way to find out what the pop and push values should be for defining a new instruction with def_op? I am running into issues finding the correct pop and push values for the new CALL_KW op.

Hi @2elli :

For CALL_KW, use -2 for pop and 1 for push. -2 indicates that the "pop" value is not fixed and indeterminate. Down the line, I will define a literal value for that to make this more clear. This code goes back to Python 2.3 where literal values did not exist.

@2elli
Copy link
Contributor Author

2elli commented Oct 11, 2024

@rocky Is there a best way to find out what the pop and push values should be for defining a new instruction with def_op? I am running into issues finding the correct pop and push values for the new CALL_KW op.

Hi @2elli :

For CALL_KW, use -2 for pop and 1 for push. -2 indicates that the "pop" value is not fixed and indeterminate. Down the line, I will define a literal value for that to make this more clear. This code goes back to Python 2.3 where literal values did not exist.

That makes sense thank you. I just pushed those changes. This does still raise an error with make check in pytest/test_stack_effect.py with error AssertionError: 57 (CALL_KW) needs adjusting; should be: should have effect -2. Is this okay to ignore or does this test need to be changed maybe?

@rocky
Copy link
Owner

rocky commented Oct 11, 2024

This does still raise an error with make check in pytest/test_stack_effect.py with error AssertionError: 57 (CALL_KW) needs adjusting; should be: should have effect -2. Is this okay to ignore or does this test need to be changed maybe?

I'll look into this when I get a chance. There is probably some (or a lot) of cleanup needed around push and pop fields.

@2elli
Copy link
Contributor Author

2elli commented Oct 11, 2024

This does still raise an error with make check in pytest/test_stack_effect.py with error AssertionError: 57 (CALL_KW) needs adjusting; should be: should have effect -2. Is this okay to ignore or does this test need to be changed maybe?

I'll look into this when I get a chance. There is probably some (or a lot) of cleanup needed around push and pop fields.

Ok sounds good thanks. Let me know if I can help out with anything there.

Copy link
Owner

@rocky rocky left a comment

Choose a reason for hiding this comment

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

This "draft" has been going on for quite a while. So let's merge. There can be other changes down the line.

@rocky rocky marked this pull request as ready for review November 8, 2024 14:07
@rocky rocky merged commit 6e8f85b into rocky:master Nov 8, 2024
6 checks passed
@2elli
Copy link
Contributor Author

2elli commented Nov 9, 2024

@rocky sounds good! I'll continue some work on this; currently there are some breaking changes I made in this. 3.12 line numbers are not correct at the moment unless ran from 3.12. I've been trying to debug in my free time. I'll update you.

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.

2 participants