search

astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
32.06k stars 1.07k forks source link

[red-knot] remove Type::Unbound #13671

Open carljm opened 2 weeks ago

carljm commented 2 weeks ago

We should not represent "unbound" as a type, because it's only meaningful within a scope, it shouldn't "travel" across scopes as a type.

Instead, we should explicitly use the use-def map methods to check whether a name may be unbound in the places where it matters.

MichaReiser commented 1 week ago

Depends on https://github.com/astral-sh/ruff/pull/13563

MichaReiser commented 1 week ago

Some extra notes from my conversation with @carljm the other day regarding on how this should be implemented

In general, when we query the type of a symbol from another scope, and that symbol either doesn't exist or is definitely unbound, that should be a diagnostic and we should return Unknown. if the symbol is conditionally bound, we should just return the type from the bound paths (and possibly a diagnostic in this case as well) the "diagnostic" part may require some of these methods to gain a more complex Outcome-style return type but it would also be possible to remove Unbound (and just return Unknown in those cases) without handling the diagnostic part at all yet

carljm commented 5 days ago

In the context of reviewing and landing #13563 I ended up looking at this a bit; not far enough to have any useful code, just enough to have more clear ideas of how we might want to implement it.

I think ultimately symbol_ty_by_id and all the functions based on it will need a more complex return type than just a Type, because callers will want to know both the type, if any, and whether the symbol is possibly unbound or definitely unbound (in which case there will be no Type available) and also possibly/definitely undeclared.

This will bring these symbol lookup functions more inline with Type::call and friends, where we use a bespoke return type (CallOutcome in the case of Type::call) to give callers more flexibility in handling various possible cases. (The root cause of the Type::Unbound mistake was me trying too hard to avoid such custom return types and squeeze too much information into a plain Type return.)

It's possible that for best efficiency this return type (we could call it SymbolLookupResult, unless we can think of something shorter that's equally clear?) should actually be a lazy wrapper around an iterator of types that need to be unioned, generic over the complex iterators constructed in bindings_ty and declarations_ty (or both chained together, in the case of a partially-declared symbol). The advantage of this would be that the caller could decide (via methods on SymbolLookupResult) to insert more types into the union, depending on maybe-unboundness, before the union is even first constructed, and we'd construct fewer temporary union types.

Or another approach that might be simpler (fewer lifetimes?), but would mean collecting the types into a vector, would be for SymbolLookupResult to effectively be a wrapper around a UnionBuilder plus bound-ness / declared-ness info, that only actually builds the union type on demand.