[red-knot] More precise inference for classes with non-class metaclasses

[InSyncWithFoo]

Summary

Resolves #14208.

Test Plan

Markdown tests.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[carljm]

Looks great, thank you! Will apply my diagnostic wording nit and then merge.

[carljm]

Oops, looks like this needs a rebase before it can land. I don't have time to do that right now, but will merge if you're able to do it.

[carljm]

Sorry, I should have clarified that it's not just a rebase -- the rebase causes logical conflicts that make compilation fail.

[InSyncWithFoo]

[...] the rebase causes logical conflicts that make compilation fail.

Indeed, now there's a panic that I can't make heads or tails of:

metaclass.md - Non-class

  crates/red_knot_python_semantic/resources/mdtest/metaclass.md:169 panicked at /home/runner/.cargo/git/checkouts/salsa-61760caba2b17ca5/88a1d77/src/runtime.rs:335:13

Here's the code in question:

def f(*args, **kwargs) -> int: ...  # 169 (not even modified)

class A(metaclass=f): ...           # 171 (also not modified)

I'm also getting "Module `knot_extensions` has no member `static_assert`" errors, but only locally (Windows). Does the new Python API require some kind of configuration?

[carljm]

now there's a panic

I can repro this, will take a couple minutes to look at it and see if I can understand it.

I'm also getting "Module `knot_extensions` has no member `static_assert`" errors, but only locally (Windows). Does the new Python API require some kind of configuration?

No, but I suspect this is related to the use of a symlink to include knot_extensions.pyi in typeshed; we should probably just copy instead. Cc @sharkdp

[carljm]

The panic is due to a Salsa query cycle. Still looking into why it's new after the rebase.

[sharkdp]

No, but I suspect this is related to the use of a symlink to include knot_extensions.pyi in typeshed; we should probably just copy instead. Cc @sharkdp

Strange that our CI tests on Windows seem to have no problem with it? I can take a look tomorrow.

[carljm]

Ok, I tracked down the cycle. It's new with the rebase because we are now doing proper call binding. What happens is that we call the metaclass f, which takes *args, and the behavior of call binding is to create a bound type for the variadic parameter *args which is the union of the types of all arguments which map to that variadic parameter. In this case that is all three of self_ty, bases, and namespace. In constructing that union, we end up checking whether namespace (which is dict) and self_ty (which is the class A) are subtypes of each other, and this check ends up needing to know the metaclass of A, which obviously leads to a cycle.

I think this reveals a bug in the PR that I hadn't noticed. The first argument to the metaclass is not the actual class object, as modeled in the PR via self_ty. That would be pretty circular at runtime, too, since the metaclass is responsible for creating the class object! The first argument to the metaclass is a string, the name of the new class:

>>> def f(*a):
...     print(a)
...
>>> class A(metaclass=f): ...
...
('A', (), {'__module__': '__main__', '__qualname__': 'A', '__firstlineno__': 1, '__static_attributes__': ()})

If we correctly reflect this by passing Type::string_literal(db, self.name(db)) as the first argument instead of Type::class_literal(self), the cycle goes away and the tests pass.

[carljm]

For future reference, the technique I found most effective for tracking down the cause of the cycle was simply adding dbg! statements in the red-knot code to bisect where the panic occurred (I found it occurred in the metaclass.call(...) line in the non-class metaclass branch of Class::try_metaclass, then I added dbg! statements inside Type::call and discovered it was happening inside bind_call, then I added dbg! statements inside bind_call until I figured out that it was happening when adding the third argument to the union of argument types for *args, etc.)

Using query tracing (in red-knot or Salsa) was ineffective in this case, because the cycle was a single-query cycle: it was just Class::try_metaclass calling itself; there were some intervening function call layers, but no intervening Salsa queries.