Skip to content

bpo-41987: Fix unnecessary evaluation of return type forward declaratons in singledispatch.#23216

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
Closed
mental32 wants to merge 2 commits into from
Closed

bpo-41987: Fix unnecessary evaluation of return type forward declaratons in singledispatch. #23216

mental32 wants to merge 2 commits into from

Conversation

@mental32
Copy link
Contributor

@mental32mental32 commented Nov 10, 2020•
edited by bedevere-bot
Loading

I know that interest was expressed in the bpo thread about finding the root cause of this issue sadly I have not been able to figure that out yet, instead I offer a patch 😅

https://bugs.python.org/issue41987

gvanrossum
gvanrossum reviewed Nov 11, 2020
View reviewed changes
Copy link
Member

@gvanrossumgvanrossum left a comment

Choose a reason for hiding this comment

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

I’m also still very interested in the root cause.

@mental32
Copy link
ContributorAuthor

mental32 commented Nov 11, 2020•
edited
Loading

@gvanrossum thank you for the review!

I've applied and committed the suggested changes, and will be investigating the regression further when I can.

At the moment I am only comparing diffs of tag 3.8 and tag 3.9 for file /Lib/typing.py since I suspect the regression will be visible there. If I can't eyeball it I'll start bisecting commits from 3.8 to master's head testing for reproducibility.

@hongweipeng
Copy link
Contributor

hongweipeng commented Nov 12, 2020

I think the reason is that ForwardRef._evaluate() did two evaluations or the recursive check failed.

@github-actions
Copy link

github-actionsbot commented Dec 16, 2020

This PR is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actionsgithub-actionsbot added the stale Stale PR or inactive for long period of time. label Dec 16, 2020
@glyphglyphmannequin mentioned this pull request Apr 10, 2022
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossumgvanrossumgvanrossum left review comments

@rhettingerrhettingerAwaiting requested review from rhettingerrhettinger is a code owner

Assignees

No one assigned

Labels

awaiting reviewstaleStale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mental32@hongweipeng@gvanrossum@the-knights-who-say-ni@bedevere-bot