Skip to content

Conversation

Tpt
Copy link
Contributor

@Tpt Tpt commented Sep 4, 2025

Create a new ABC class and make the relevant classes virtual subclasses of it

This way there is no type implementing PyTypeCheck and not PyTypeInfo

Create a new ABC class and make the relevant classes virtual subclasses of it
@Tpt Tpt force-pushed the tpt/weakref-type-info branch from 8f94d51 to 4e4cbb4 Compare September 4, 2025 08:05
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof, I am really unsure about this. The fact that we have to invent an ABC here feels pretty awkward, I think that would probably surprise users quite a lot.

I think we should consider whether:

  • This is a suggestion that we can't fully remove PyTypeCheck, and we should re-plan, or
  • We should remove PyWeakref and just have the two dedicated types.
    • Even if we do that, we need to figure out how to make PyWeakrefProxy compatible with PyTypeInfo. Maybe there's a split that's not PyTypeInfo / PyTypeCheck but a different split where there's a trait which exposes an "object" used for isinstance (which might be a tuple or union) and a second trait which guarantees that object is a single concrete type object?

Comment on lines +21 to +32
TYPE.get_or_try_init(py, || {
// Terrible way to generate a super class of both ProxyType and CallableProxyType
let globals = PyDict::new(py);
globals.set_item("abc", py.import("abc")?)?;
py.run(ffi::c_str!("class WeakrefProxy(abc.ABC):\n pass"), Some(&globals), None)?;
let cls = globals.get_item("WeakrefProxy")?.downcast_into::<PyType>()?;
let weakref = py.import("weakref")?;
for child_name in ["ProxyType", "CallableProxyType"] {
cls.call_method1("register", (weakref.getattr(child_name)?,))?;
}
PyResult::Ok(cls.unbind())
}).unwrap().bind(py).as_type_ptr()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I wonder if we could use weakref.ProxyTypes. That might require logic to allow the "type object" to not be a real type object but also a tuple (or union)? That would break other behaviour about PyTypeInfo, however.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this badly breaks a lot of assumptions because type_object returns a PyType

@Tpt
Copy link
Contributor Author

Tpt commented Sep 4, 2025

Yes, this is very hackward and hacky.

Yes, the real problem is PyWeakrefProxy that has no match in Python.

Maybe there's a split that's not PyTypeInfo / PyTypeCheck but a different split where there's a trait which exposes an "object" used for isinstance (which might be a tuple or union) and a second trait which guarantees that object is a single concrete type object?

This sounds a pretty good way forward. We can add a super_type_object method to PyTypeCheck with a clear documentation. It would unblock #5387 fairly nicely. I will give it a try.

@Tpt
Copy link
Contributor Author

Tpt commented Sep 5, 2025

Other attempt in #5387 that introduces PyTypeCheck::classinfo_object

@Tpt Tpt closed this Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants