Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

TypeOffset class no longer depends on target Python version#1292

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

lostmsu
Copy link
Member

This is a stage of removing the need to prepare a separate build ofPython.Runtime for each Python version.

What does this implement/fix? Explain your changes.

This removes the need to pickTypeOffset at compile time. Instead, for each supported Python version a separate class is generated (e.g.TypeOffset36). Then theRuntime (via new classABI) picks the correct class based on actual Python version using reflection, and copies only the necessaryTypeOffset members over from it.

ManagedDataOffsets.Magic is now also read at runtime fromtp_basicsize of thebuiltins.type class.

Any other comments?

N/A

Checklist

Check all those that are applicable and complete.N/A

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Add yourself toAUTHORS
  • Updated theCHANGELOG

filmor reacted with rocket emoji
@lostmsulostmsu requested a review fromfilmorNovember 24, 2020 08:32
@@ -351,6 +350,10 @@ def main():
gen_interop_head(writer)

gen_heap_type_members(ast_parser, writer)

ver_define = "PYTHON{0}{1}".format(PY_MAJOR, PY_MINOR)
writer.append(code="#if " + ver_define)
Copy link
Member

Choose a reason for hiding this comment

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

Why are these still behind a compile-time flag? Do you plan to change that?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Yeah, I am approaching this gradually.

Copy link
MemberAuthor

@lostmsulostmsuNov 30, 2020•
edited
Loading

Choose a reason for hiding this comment

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

I checked this out, and the rest of the conditional stuff came from1b466df. Since it was only used to determine if methods in Python.Runtime itself implement slots (which we can do on our own), I took a liberty to mostly revert it in this PR. Reversal is inthis commit

@lostmsulostmsuforce-pushed thefeatures/VersionIndependent branch 3 times, most recently fromd6f4834 to3bc282aCompareNovember 25, 2020 02:25
@codecov-io
Copy link

codecov-io commentedNov 25, 2020•
edited
Loading

Codecov Report

Merging#1292 (f9a3a53) intomaster (182faed) willnot change coverage.
The diff coverage isn/a.

Impacted file tree graph

@@           Coverage Diff           @@##           master    #1292   +/-   ##=======================================  Coverage   87.97%   87.97%           =======================================  Files           1        1             Lines         291      291           =======================================  Hits          256      256             Misses         35       35
FlagCoverage Δ
setup_linux65.29% <ø> (ø)
setup_windows74.22% <ø> (ø)

Flags with carried forward coverage won't be shown.Click here to find out more.

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last update182faed...f9a3a53. Read thecomment docs.

@lostmsu
Copy link
MemberAuthor

Hm, Travis build on Linux with Python 3.9 regularly fails with this change. Could I have screwed up something here?@filmor, you've recently added support for 3.9. Was there anything specific to Linux?

@filmor
Copy link
Member

Nope, the only reason why support for 3.9 took so long was that I had to adjust the interop generation as it wasn't working with the new header-files anymore.

@lostmsulostmsu reopened thisNov 25, 2020
@lostmsu
Copy link
MemberAuthor

lostmsu commentedNov 25, 2020•
edited
Loading

@filmor , oh, nvm. Looks like Travis just used older commit for 3.9 o-O

It passes now.

@lostmsulostmsu requested a review fromfilmorNovember 28, 2020 22:04
Copy link
Member

@filmorfilmor left a comment

Choose a reason for hiding this comment

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

I think this is fine for now, just minor things that I think could be fixed.

@lostmsulostmsuforce-pushed thefeatures/VersionIndependent branch from3bc282a toa2c13c7CompareNovember 29, 2020 22:08
Instead, for each supported Python version a separate class is generated (e.g. TypeOffset36). Then the Runtime picks the correct class using reflection, and copies only the necessary TypeOffset members over from it.ManagedDataOffsets.Magic is now also read at runtime from tp_basicsize of PyType
@lostmsulostmsuforce-pushed thefeatures/VersionIndependent branch froma2c13c7 toe36a027CompareNovember 29, 2020 22:09
@lostmsulostmsu requested a review fromfilmorNovember 29, 2020 22:48
@lostmsulostmsu merged commitab97b02 intopythonnet:masterNov 30, 2020
@lostmsulostmsu deleted the features/VersionIndependent branchNovember 30, 2020 18:36
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@filmorfilmorAwaiting requested review from filmor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@lostmsu@codecov-io@filmor
[8]ページ先頭
©2009-2025 Movatter.jp