fix type confusion when different #[pyclass] types returned from #[new]#6062
fix type confusion when different #[pyclass] types returned from #[new]#6062davidhewitt wants to merge 4 commits into
#[pyclass] types returned from #[new]#6062Conversation
|
Thanks, will take a look in the coming days. |
davidhewitt
left a comment
There was a problem hiding this comment.
No worries, I am not sure how much I will get done before Sunday at the earliest myself.
| /// This resolution step is necessary in order to encode the preference for `PyClassInitializer<T>` | ||
| /// to use proper machinery instead of `IntoPyObject`; if we removed the implementation of | ||
| /// `IntoPyObject` for `PyClassInitializer<T>` then this machinery could probably collapse away to | ||
| /// type inference. |
There was a problem hiding this comment.
This is not true, I was confused when writing this due to conflicting implementations of an earlier draft of my changes PyClassInit. I think really what's going on is that it's legal for users to add implementations of IntoPyObject for PyClassInitializer<UserType>? Will need to think about this again.
| { #pyo3_path::impl_::pyclass::IsPyClass::<#output>::VALUE }, | ||
| { #pyo3_path::impl_::pyclass::IsInitializerTuple::<#output>::VALUE } |
There was a problem hiding this comment.
For the record, the original bug was that these specializations apply for all pyclasses, not just the #[pyclass] whose #[new] we are currently implementing, so we were running the specialized conversion for the wrong type.
This fixes a bug where a
#[pyclass] struct Bcould be returned from the#[new]implementation for a separate#[pyclass] struct A(essentially instantly crashes).The newly added test in
test_class_init.rsis an example of what currently fails onmain.I moved the implementation of
tp_new_impltosrc/internalas it doesn't need to be publicly reachable, and used deref specialization to buildPyClassInitializer<T>if possible inside#[new]implementation ofT.While I was at it, I also needed some new ok-wrapping machinery (to avoid needing my trait shenanigans to handle results) and also improved the diagnostics for incorrect return types (see new UI test).
cc @Icxolu
(Credit to Codex security scanning for the bug discovery; solution is my own design.)