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

gh-116126: Implement PEP 696#116129

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
JelleZijlstra merged 56 commits intopython:mainfromJelleZijlstra:pep696v2
May 3, 2024
Merged

Conversation

JelleZijlstra
Copy link
Member

@JelleZijlstraJelleZijlstra commentedFeb 29, 2024
edited
Loading

This implements both the grammar and compiler changes and changes totyping.py behavior (the latter copied from typing-extensions).

Gobot1234, m-aciek, and ditansu reacted with heart emoji
@JelleZijlstra
Copy link
MemberAuthor

This should be ready for review now, I'll take it out of draft once the tests pass. Notes:

  • This PR intentionally does not cover documentation or typing.py behavior; I'll leave that to another PR.
  • The PEP 695 test suite helped me find some very interesting issues with the symtable effects of this change. I think the current solution should work, but definitely didn't expect that to be the trickiest aspect of the change.
  • It's unfortunate that the new AST field is calleddefault_ in Python. It can't bedefault in C because that's a keyword. I may need to change the AST generation machinery to support aliasing a field so the Python-visible name can be just "default".

@pablogsal
Copy link
Member

Will review this weekend

JelleZijlstra reacted with heart emoji

@JelleZijlstra
Copy link
MemberAuthor

@pablogsal have you had a chance to look at this? The grammar changes should be quite straightforward, so hopefully it's not complicated to review.

I'll spend some time today trying to make it so the field is called "default" not "default_" in Python, because having "default_" in the Python-visible APIs would be ugly.

@bedevere-botbedevere-bot removed the 🔨 test-with-refleak-buildbotsTest PR w/ refleak buildbots; report in status section labelApr 23, 2024
JelleZijlstraand others added2 commitsApril 28, 2024 13:30
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Copy link
Member

@AlexWaygoodAlexWaygood left a comment

Choose a reason for hiding this comment

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

I would say "is not thetyping.NoDefault singleton" rather than "is not equal totyping.NoDefault", since we usually argue that an identity check is more idiomatic for singleton objects such asNone,NotImplemented andEllipsis

Comment on lines +3244 to +3248
>>> T.__default__ is typing.NoDefault
True
>>> S = TypeVar("S", default=None)
>>> S.__default__ is None
True
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
>>>T.__default__is typing.NoDefault
True
>>>S= TypeVar("S",default=None)
>>>S.__default__isNone
True
>>>T.has_default()
False
>>>T.__default__is typing.NoDefault
True
>>>S= TypeVar("S",default=None)
>>>S.has_default()
True
>>>S.__default__isNone
True

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I thought of that but it felt out of place in the docs forNoDefault, since the cases you added don't useNoDefault at all.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough -- I thought it was quite nice to see together in one example how the two concepts interrelate, but I definitely don't feel strongly!

JelleZijlstraand others added2 commitsApril 28, 2024 14:29
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@JelleZijlstra
Copy link
MemberAuthor

@ericsnowcurrently the C globals check is failing because this PR now adds a new static type and global singleton,typing.NoDefault (closely analogous tobuiltins.NotImplemented). What do you think we should do? We could simply allowlist the new globals, or somehow makeNoDefault not rely on C globals—but the latter feels like it would be a lot more complicated.

@ericsnowcurrently
Copy link
Member

For now let's just whitelist the new singleton, as long as it is stateless and immortal. We can circle back later if the whitelist is a problem.

JelleZijlstra reacted with thumbs up emoji

@JelleZijlstraJelleZijlstra added the 🔨 test-with-refleak-buildbotsTest PR w/ refleak buildbots; report in status section labelApr 30, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@JelleZijlstra for commit5f6fdfd 🤖

If you want to schedule another build, you need to add the🔨 test-with-refleak-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-refleak-buildbotsTest PR w/ refleak buildbots; report in status section labelApr 30, 2024
The previous logic would add the size of the pointer target to the pointer value, which might point to another pointer being used as a scope key. Now, we increment the value as an integer instead, which means it can never be a valid pointer.
@JelleZijlstraJelleZijlstra merged commitca269e5 intopython:mainMay 3, 2024
@JelleZijlstraJelleZijlstra deleted the pep696v2 branchMay 3, 2024 13:17
SonicField pushed a commit to SonicField/cpython that referenced this pull requestMay 8, 2024
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
@JelleZijlstraJelleZijlstra restored the pep696v2 branchSeptember 10, 2024 23:37
@JelleZijlstraJelleZijlstra deleted the pep696v2 branchSeptember 24, 2024 15:40
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@carljmcarljmcarljm left review comments

@picnixzpicnixzpicnixz left review comments

@hauntsaninjahauntsaninjahauntsaninja left review comments

@erlend-aaslanderlend-aaslanderlend-aasland left review comments

@AlexWaygoodAlexWaygoodAlexWaygood approved these changes

@isidenticalisidenticalAwaiting requested review from isidenticalisidentical is a code owner

@pablogsalpablogsalAwaiting requested review from pablogsalpablogsal is a code owner

@lysnikolaoulysnikolaouAwaiting requested review from lysnikolaoulysnikolaou is a code owner

@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner

@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner

@iritkatrieliritkatrielAwaiting requested review from iritkatrieliritkatriel is a code owner

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

Successfully merging this pull request may close these issues.

9 participants
@JelleZijlstra@pablogsal@AlexWaygood@bedevere-bot@ericsnowcurrently@carljm@picnixz@hauntsaninja@erlend-aasland

[8]ページ先頭

©2009-2025 Movatter.jp