Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Aug 6, 2021
Merged

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

merged 4 commits into from
Aug 6, 2021

Conversation

@BvB93
Copy link
Contributor

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

An example from prior to this PR:

>>> from typing import Union
>>> print(Union[int, float].__name__)
None

https://bugs.python.org/issue44524

@BvB93 BvB93 requested a review from gvanrossum as a code owner August 5, 2021 16:16
@Fidget-Spinner Fidget-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
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

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.

@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
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

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

Great. Thanks!

@ambv
Copy link
Contributor

ambv commented Aug 6, 2021

GHA Windows 32-bit failure in asyncio unrelated.

@ambv ambv merged commit 8bdf12e into python:main Aug 6, 2021
@miss-islington
Copy link
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
Copy link

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

@bedevere-bot bedevere-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._name was 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
Copy link
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
Copy link
Contributor

ambv commented Aug 7, 2021

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

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

Reviewers

@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes

@gvanrossum gvanrossum Awaiting requested review from gvanrossum

Assignees

No one assigned

Labels

type-bug An 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