Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recursively visit deferred AST nodes #9541

Merged
merged 1 commit into from
Jan 16, 2024
Merged

Recursively visit deferred AST nodes #9541

merged 1 commit into from
Jan 16, 2024

Conversation

charliermarsh
Copy link
Member

Summary

This PR is a more holistic fix for #9534 and #9159.

When we visit the AST, we track nodes that we need to visit later (deferred nodes). For example, when visiting a function, we defer the function body, since we don't want to visit the body until we've visited the rest of the statements in the containing scope.

However, deferred nodes can themselves contain deferred nodes... For example, a function body can contain a lambda (which contains a deferred body). And then there are rarer cases, like a lambda inside of a type annotation.

The aforementioned issues were fixed by reordering the deferral visits to catch common cases. But even with those fixes, we still fail on cases like:

from __future__ import annotations

import re
from typing import cast

cast(lambda: re.match, 1)

Since we don't expect lambdas to appear inside of type definitions.

This PR modifies the Checker to keep visiting until all the deferred stacks are empty. We already do this for any one kind of deferred node; now, we do it for all of them at a level above.

@charliermarsh charliermarsh changed the title Charlie/deferred Recursively visit deferred AST nodes Jan 16, 2024
@charliermarsh charliermarsh added bug Something isn't working internal An internal refactor or improvement and removed internal An internal refactor or improvement labels Jan 16, 2024
Copy link

codspeed-hq bot commented Jan 16, 2024

CodSpeed Performance Report

Merging #9541 will improve performances by 4.55%

Comparing charlie/deferred (ef91b4f) with main (da275b8)

Summary

⚡ 1 improvements
✅ 29 untouched benchmarks

Benchmarks breakdown

Benchmark main charlie/deferred Change
parser[numpy/ctypeslib.py] 12.8 ms 12.2 ms +4.55%

Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@charliermarsh charliermarsh merged commit f9331c7 into main Jan 16, 2024
17 checks passed
@charliermarsh charliermarsh deleted the charlie/deferred branch January 16, 2024 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant