Skip to content

bpo-44524: Fix an issue wherein _GenericAlias._name was not properly set for specialforms - #27614

Merged
ambv merged 4 commits into
python:mainfrom
BvB93:name
Aug 6, 2021
Merged

bpo-44524: Fix an issue wherein _GenericAlias._name was not properly set for specialforms#27614
ambv merged 4 commits into
python:mainfrom
BvB93:name

Conversation

@BvB93

@BvB93BvB93 commented Aug 5, 2021•
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

An example from prior to this PR:

>>>fromtypingimportUnion>>>print(Union[int, float].__name__)
None

https://bugs.python.org/issue44524

@BvB93
BvB93 requested a review from gvanrossum as a code ownerAugust 5, 2021 16:16
@Fidget-SpinnerFidget-Spinner added needs backport to 3.10 only security fixes type-bug An unexpected behavior, bug, or error labels Aug 5, 2021
Fidget-Spinner
Fidget-Spinner reviewed Aug 5, 2021
View reviewed changes

@Fidget-SpinnerFidget-Spinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM in general. Thanks for the quick fix :). I just have a few minor comments below.

Comment threadLib/test/test_typing.py
Comment threadMisc/NEWS.d/next/Library/2021-08-05-18-20-17.bpo-44524.9T1tfe.rst Outdated
Comment threadLib/typing.py Outdated
@BvB93@Fidget-Spinner
Apply suggestions from code review
ab3a89f
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
Fidget-Spinner
Fidget-Spinner approved these changes Aug 5, 2021
View reviewed changes

@Fidget-SpinnerFidget-Spinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great. Thanks!

@ambv

ambv commented Aug 6, 2021

Copy link
Copy Markdown
Contributor

GHA Windows 32-bit failure in asyncio unrelated.

@ambv
ambv merged commit 8bdf12e into python:mainAug 6, 2021
@miss-islington

miss-islington commented Aug 6, 2021

Copy link
Copy Markdown
Contributor

Thanks @BvB93 for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 6, 2021
@BvB93@miss-islington
bpo-44524: Fix an issue wherein _GenericAlias._name was not properl…
7311293
…y set for specialforms (pythonGH-27614)
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
(cherry picked from commit 8bdf12e)
Co-authored-by: Bas van Beek <43369155+BvB93@users.noreply.github.com>
@bedevere-bot

bedevere-bot commented Aug 6, 2021

Copy link
Copy Markdown

GH-27632 is a backport of this pull request to the 3.10 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.10 only security fixes label Aug 6, 2021
@BvB93
BvB93 deleted the name branch August 6, 2021 13:40
miss-islington added a commit that referenced this pull request Aug 6, 2021
@miss-islington@BvB93
bpo-44524: Fix an issue wherein_GenericAlias._namewas not properl…
36a2497
…y set for specialforms (GH-27614)
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
(cherry picked from commit 8bdf12e)
Co-authored-by: Bas van Beek <43369155+BvB93@users.noreply.github.com>
@pablogsal

pablogsal commented Aug 6, 2021

Copy link
Copy Markdown
Member

Unfortunately this PR has introduced reference leaks:

❯ ./python -m test test_typing -R :
0:00:00 load avg: 1.12 Run tests sequentially
0:00:00 load avg: 1.12 [1/1] test_typing
beginning 9 repetitions
123456789
.........
test_typing leaked [29, 29, 29, 29] references, sum=116
test_typing leaked [10, 10, 10, 10] memory blocks, sum=40
test_typing failed (reference leak)
== Tests result: FAILURE ==
1 test failed:
test_typing
1 re-run test:
test_typing
Total duration: 1.2 sec
Tests result: FAILURE

@ambv

ambv commented Aug 7, 2021

Copy link
Copy Markdown
Contributor

Turns out this PR discovered refleaks that were there all along 😎

@tk0miyatk0miya mentioned this pull request Aug 10, 2021
Closed
@farcatfarcatmannequin mentioned this pull request Apr 14, 2022
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Fidget-SpinnerFidget-SpinnerFidget-Spinner approved these changes
@gvanrossumgvanrossumAwaiting requested review from gvanrossum

Assignees

No one assigned

Labels

type-bugAn unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@BvB93@ambv@miss-islington@bedevere-bot@pablogsal@Fidget-Spinner@the-knights-who-say-ni