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

[red-knot] model that class/comprehension bodies run immediately #13933

Closed
carljm opened this issue Oct 25, 2024 · 7 comments
Closed

[red-knot] model that class/comprehension bodies run immediately #13933

carljm opened this issue Oct 25, 2024 · 7 comments
Assignees
Labels
red-knot Multi-file analysis & type inference

Comments

@carljm
Copy link
Contributor

carljm commented Oct 25, 2024

Currently, we have a general strategy for name lookups in enclosing scopes: they use the type as seen from end of the scope where it's defined. For example:

x = 1

def f():
    # This is correct for any call to `f` that would happen after this module has
    # fully executed. It could be wrong if `f` is called during execution of the
    # module body.
    reveal_type(x)  # revealed: Literal["foo"]

x = "foo"

For nested functions, this is a pretty reasonable default behavior. Another option would be to consider all definitions of x (or at least all definitions of x visible from the point where f is defined, forwards). This would result in revealing Literal[1] | Literal["foo"] for x in the inner scope of f, instead. This would be safer, but also more annoying for the more common case where f is not called from the module body. It would also mean we would always consider x possibly-undefined inside f, if f is defined before x, which could be really annoying.

We could try to be smarter by detecting, for example, that the function f is (or might be) called from module level, and only consider earlier definitions if we observe this possibility. But this will always be best-effort, since reliably detecting all possible paths (other functions, even possibly in other modules) through which f could be called from module level isn't feasible.

But there are cases where we can be quite sure that the nested scope will run before the end of the outer scope. For example, class definitions:

x = 1
class C:
    y = x
x = "foo"

# This is currently what we reveal, but it's clearly wrong: should be `Literal[1]`
reveal_type(C.y)  # revealed: Literal["foo"]

The class body always runs immediately, so it should resolve types in the enclosing scope from the class definition's point in control flow, not from end-of-scope.

@carljm carljm added the red-knot Multi-file analysis & type inference label Oct 25, 2024
@AlexWaygood
Copy link
Member

Should we do the same for list/dict/set comprehensions? (Generators expressions obviously do not run immediately, unlike list/dict/set comprehensions.)

@carljm
Copy link
Contributor Author

carljm commented Oct 26, 2024

Yes!

@carljm carljm changed the title [red-knot] model that class bodies run immediately [red-knot] model that class/comprehension bodies run immediately Oct 26, 2024
@carljm carljm added this to the Red Knot 2024 milestone Nov 7, 2024
@carljm carljm modified the milestones: Red Knot 2024, Red Knot Q1 2025 Jan 9, 2025
@carljm
Copy link
Contributor Author

carljm commented Jan 9, 2025

See #13879 for discussion of generator expressions as it relates to this task.

@dcreager dcreager self-assigned this Feb 10, 2025
@dcreager
Copy link
Member

#16079

@MichaReiser
Copy link
Member

@dcreager is this complete now?

@carljm
Copy link
Contributor Author

carljm commented Feb 19, 2025

I think it is.

@carljm carljm closed this as completed Feb 19, 2025
@dcreager
Copy link
Member

I'm looking at how we're representing the bindings snapshots next, to see if there's any performance we can squeeze out of it. But I'm 👍 to closing this since the functionality is there and working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
red-knot Multi-file analysis & type inference
Projects
None yet
Development

No branches or pull requests

4 participants