- Notifications
You must be signed in to change notification settings - Fork1.4k
New subclass payload layout#6319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for usingCodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| #[inline] | ||
| fnpayload_type_id() -> std::any::TypeId{ | ||
| std::any::TypeId::of::<PyInt>() | ||
| } |
youknowoneDec 2, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is okay here, because PyInt and PyBool shares same payload. The only difference is type.
But implementing this makesPyPayload::downcastable to be automaticallytrue for any kind ofPyInt. It doesn't make failing tests on this patch because we don't use that pattern. But it obviously will be a sort of trap for future developing.
Another question is thatPyPayload::downcastable only takes &PyObject currently. We can't compare it to actual type... because we don't have vm.ctx.
| #[pyclass(name ="bool", module =false, base =PyInt)] | ||
| pubstructPyBool; | ||
| #[repr(transparent)] | ||
| pubstructPyBool(pubPyInt); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Now we have to manually define the base layout to be always leading field.
The problem is Rust actually only guarantees it to be actually same layout only when #[repr(transparent)] is used.
What I didn't check yet: Can using #[repr(C)] helps? Is there any other repr options to keep the first field as same as its original layout?
I am considering new subclass payload layout to provide
I will left open questions on the code.