Skip to content

[red-knot] Remove Definition from Class and FunctionType #14054

@MichaReiser

Description

@MichaReiser

The ClassType and FunctionType both store a back reference to their origin definition, and the definition holds on to the AST node. This is problematic because Types are visible cross-module:

  • If a Type changes (no longer compares equal), it requires redoing type inference for all dependent types (modules). This gets worse because Types flow and compose. E.g., every union containing a ClassType would have to be thrown away if the node's AST location changes (and, in turn, all results that depend on that Union...). Fortunately, this currently works thanks to Salsa tracking each struct field on its own. However, it will break if we change to use coarse-grained salsa dependencies for better performance.
  • Incrementally checking a program after restoring from a persistent cache shouldn't require reparsing any unchanged modules. Instead, Red Knot resolves the types directly from the cache. For this to work efficiently, it's important that Types have no backreference to the AST. Otherwise, deserializing the Type requires reparsing the module or storing and deserializing the AST in/from the persistent cache. But it's not just the cost of deserialization; this also comes at a cost that Red Knot now holds on to ASTs that are never needed. It shouldn't be necessary to revisit the AST of standard library modules after they've been type-checked.

Metadata

Metadata

Assignees

Labels

tyMulti-file analysis & type inference

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions