Skip to content

Recursively visit deferred AST nodes#9541

Merged
charliermarsh merged 1 commit intomainfrom
charlie/deferred
Jan 16, 2024
Merged

Recursively visit deferred AST nodes#9541
charliermarsh merged 1 commit intomainfrom
charlie/deferred

Conversation

@charliermarsh
Copy link
Copy Markdown
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
@codspeed-hq
Copy link
Copy Markdown

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%

@github-actions
Copy link
Copy Markdown
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
@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