[red-knot] circular references in class definitions panic

[rtpg]

class C(list[C]): ...

the above code causes red_knot to panic, due to something a bit subtle:

• We query for the type of C via infer_definition_types
• during this process we infer the type of list[C]
• during that process we (correctly!) annotate the type of list and stuff it in the inference expressions
• We then try to get the type of C... and call infer_definition_types

This leads to C being infered of type Unknown thanks to infer_definition_types_cycle_recover, which is fine and great... but due to something I don't quite understand, the result of infer_definition_types_cycle_recover ends up being the final definition of infer_definition_types(C).

As a result we lose the inference on list! So later linting code that traverses the AST tries to look up the type of the list expression and blows up.

My understanding here is that the queries are like:

infer_definition_types(C) -> infer_definition_types(C)
--------------------------------------------------------
infer_definition_types(C) -> [C: Unknown]
--------------------------------------------------------
[C: ClassType (base: Unknown), expressions: {list} ]

but that future queries to the DB for infer_defintion_types(C) ends up returning [C: Unknown]. I have been reading the docs on salsa cycling a bit on this point to try and decipher what is going on here but maybe somebody else would have a better understanding of what can be done here.

[MichaReiser]

@carljm's working on fixpoint iteration that will allow cyclic dependencies

[rtpg]

Alright, good to know! I had this intuition that I didn't need general fixed point iteration (just not caching the cycle recovery result for definitions), but trying to db.report_untracked_read my way out of this got me nowhere.

Will studiously wait on this one for the adults to show up with the principled solution when the time is right.

[carljm]

Salsa's current cycle handling intentionally sets the "cycle fallback" result as the result for every query in the cycle, not just the single query that finally triggered the cycle. This is done for determinism reasons (so that results don't change depending on where you happen to enter a cycle.) I think this behavior explains the result you're seeing.

For the code you posted, I would expect it to work in a stub file (where class bases are deferred) but not in a regular Python file, where they are eagerly evaluated.

Salsa's current cycle recovery is not very useful for us, which is why I'm working on the fixpoint iteration feature, which we definitely need, for example, for evaluating types in loopy control flow graphs.