◐ Shell
reader mode source ↗
Skip to content

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

Closed
mental32 wants to merge 2 commits into
python:mainfrom
mental32:bpo-41987
Closed

bpo-41987: Fix unnecessary evaluation of return type forward declaratons in singledispatch.#23216
mental32 wants to merge 2 commits into
python:mainfrom
mental32:bpo-41987

Conversation

@mental32

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

Copy link
Copy Markdown
Contributor

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

@mental32 mental32 requested a review from rhettinger as a code owner November 10, 2020 06:08
gvanrossum
gvanrossum reviewed Nov 11, 2020
View reviewed changes

@gvanrossum gvanrossum 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.

Hide comment

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

@mental32

mental32 commented Nov 11, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@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

hongweipeng commented Nov 12, 2020

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Dec 16, 2020

Copy link
Copy Markdown

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-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Dec 16, 2020
@mental32 mental32 closed this Sep 19, 2021
@glyph glyph mannequin mentioned this pull request Apr 10, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum left review comments

@rhettinger rhettinger Awaiting requested review from rhettinger

Assignees

No one assigned

Labels

awaiting review stale Stale 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