Skip to content

Fix PyLayout and restrict PyRef::as_ref for non-native types#783

Merged
kngwyu merged 3 commits intoPyO3:masterfrom
kngwyu:fix-pylayout
Mar 3, 2020
Merged

Fix PyLayout and restrict PyRef::as_ref for non-native types#783
kngwyu merged 3 commits intoPyO3:masterfrom
kngwyu:fix-pylayout

Conversation

@kngwyu
Copy link
Member

@kngwyu kngwyu commented Mar 2, 2020

Fix #782.
Simply I make get_ptr a private method of PyCellInner.
This change forbids to get a base object of #[pyclass(extends=NativeType] style pyclasses.
Current &PyAny style API uses mainly a pointer of a pointer as an object handle, and integrates really badly with new PyCell 😣

@kngwyu
Copy link
Member Author

kngwyu commented Mar 2, 2020

FYI, this is a valid implementation of get_ptr for PyAny

unsafe fn get_ptr(&self) -> *mut PyAny {
    self.py().from_borrowed_ptr(self as *const _ as *mut _) as *const PyAny as *mut _
}

... but I'm not sure we should do so.
It would work but I'm sure it's a dirty solution in that it stores the same pointer 2 times.

@kngwyu kngwyu force-pushed the fix-pylayout branch 2 times, most recently from bffbdeb to 8244d13 Compare March 2, 2020 11:06
@davidhewitt
Copy link
Member

I've had a read through and looks fine. It's a shame to lose the safe API to access the super class.

I want to think for a bit longer and see if I can figure out another way to solve this.

@kngwyu
Copy link
Member Author

kngwyu commented Mar 3, 2020

It's a shame to lose the safe API to access the super class.

Yeah, but since native type inheritance was broken in PyO3 < 0.9, it's a not breaking change and not very problematic.

@davidhewitt
Copy link
Member

Agreed. I suggest we merge this PR to fix the safety issue, and I'll create a separate issue for finding a safe API for upcasting.

@kngwyu kngwyu merged commit 3115667 into PyO3:master Mar 3, 2020
@kngwyu kngwyu deleted the fix-pylayout branch March 3, 2020 08:09
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.

Implementation of PyLayout::get_ptr for native types is broken

2 participants