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

Add python 3.11 support #514

Merged
merged 14 commits into from
Feb 21, 2023
Merged

Conversation

frmdstryr
Copy link
Contributor

@frmdstryr frmdstryr commented Dec 19, 2022

This updates the code generator/compiler to work with Python 3.11, it builds on the pegen branch because the main branch just fails due to invalid col offsets in the ast.

On a positive note I get about a 10-15% speedup with this on enaml-web :).

@frmdstryr frmdstryr changed the base branch from main to pegen-parser December 19, 2022 13:17
@frmdstryr frmdstryr changed the base branch from pegen-parser to main December 19, 2022 13:17
@frmdstryr frmdstryr marked this pull request as draft December 19, 2022 13:24
@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2022

Codecov Report

Merging #514 (57d3515) into main (052cd81) will increase coverage by 0.19%.
The diff coverage is 95.92%.

❗ Current head 57d3515 differs from pull request most recent head 7564bb3. Consider uploading reports for the commit 7564bb3 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #514      +/-   ##
==========================================
+ Coverage   73.22%   73.41%   +0.19%     
==========================================
  Files         296      296              
  Lines       25837    26001     +164     
  Branches     3650     4401     +751     
==========================================
+ Hits        18918    19088     +170     
+ Misses       5834     5826       -8     
- Partials     1085     1087       +2     

@@ -383,6 +430,9 @@ def try_squash_raise(self):
the code, rather than any function called by the code.

"""
if PY311:
yield
return # TODO: Is this still possible?
Copy link
Member

@MatthieuDartiailh MatthieuDartiailh Dec 28, 2022

Choose a reason for hiding this comment

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

Using TryBegin and TryEnd pseudo instructions this should be feasible. I can give it a try but I may prioritize getting the pegen branch in first since we need it too

""" Make a function from a code object on the TOS.

"""
if not PY311:
self.load_const(name)
self.code_ops.append( # TOS -> qual_name -> code -> defaults
bc.Instr("MAKE_FUNCTION", n_defaults), # TOS -> func
Copy link
Member

Choose a reason for hiding this comment

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

Under 3.11 MAKE_FUNCTION does not take the number of default anymore but a code (see https://docs.python.org/3.11/library/dis.html#opcode-MAKE_FUNCTION). This probably explain your issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like this has been the case ever since 2.7, anyways there is only one call to make_function with defaults and it happens to be 0x01 so that is why it worked.

Copy link
Member

Choose a reason for hiding this comment

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

I thought it could be the origin of your default argument issue. I will have to look elsewhere (possibly in the C helpers)

@MatthieuDartiailh
Copy link
Member

Could you rebase this on main now that #474 is in ? (Cherry picking your changes might be easier) Also I apologize for the last two commits, I forgot to switch branch.

@MatthieuDartiailh
Copy link
Member

So I have an implementation for try_squash_raise. I just need to audit the usages to ensure we never have a try/except inside the try_squash_raise.

I will look at the kwarg default next.

@MatthieuDartiailh
Copy link
Member

Thanks for rebasing. I will push my solution for try_squah_raise later today. The issue with function default lies in CPython itself. PyEval_EvalCodeEx started using _PyFunction_FromConstructor in 3.11 but this function does not pass the defaults found in the frame constructor... I will open an issue on CPython repo.

@MatthieuDartiailh
Copy link
Member

CPython issue reported at python/cpython#101072

@MatthieuDartiailh
Copy link
Member

Still blocked on the CPython issue, the PR I made is pending review.

@MatthieuDartiailh
Copy link
Member

The cpython issue has been closed and the fix will be in 3.11.2 ! I will try to have everything ready by then.

@MatthieuDartiailh
Copy link
Member

3.11.2 is available on GHA (I misremembered the release date), let's see if it works !

@frmdstryr
Copy link
Contributor Author

Looks like it still is using 3.11.1

@MatthieuDartiailh
Copy link
Member

Yes and I find it extremely annoying ! We will have to wait a tiny bit longer it seems.

@MatthieuDartiailh MatthieuDartiailh marked this pull request as ready for review February 21, 2023 11:44
@MatthieuDartiailh MatthieuDartiailh merged commit bd1efc9 into nucleic:main Feb 21, 2023
@MatthieuDartiailh
Copy link
Member

Thanks for your work @frmdstryr

@frmdstryr
Copy link
Contributor Author

Thank 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.

None yet

3 participants