Avoid using PyDictObject when compiling for RustPython#6057
Conversation
davidhewitt
left a comment
There was a problem hiding this comment.
Sorry for the slow review; I'm not sure this is enough:
- See e.g.
dict_lenfunction which assumes the layout ofPyDictObject; as long as the cast suceeds then this function is potentially wrong. Perhaps we should makePyDictObjectopaque (or not exposed at all) inpyo3-ffiunder RustPython? - I haven't checked extensively, but I have similar concerns for other types. e.g.
PyTupleMethods::as_slice.
26f0cab to
fef3a8c
Compare
RustPython will never support compiling without the abi3 feature enabled. So PyDictObject will never be available. I can look into enforcing the |
|
In 55c2ea9, I force enable the |
55c2ea9 to
adb884c
Compare
| // If Py_GIL_DISABLED is set, do not build with limited API support. | ||
| if (self.abi3 && !self.is_free_threaded()) | ||
| || self.implementation == PythonImplementation::RustPython | ||
| { |
There was a problem hiding this comment.
Thanks, this looks good. cc @ngoldbaum for interaction with #5786, I guess we won't need the is_free_threaded exclusion after that? Which PR should we merge first?
| #[cfg(not(Py_LIMITED_API))] | ||
| #[cfg(not(any(Py_LIMITED_API, RustPython)))] | ||
| mod cpython; | ||
|
|
||
| #[cfg(not(Py_LIMITED_API))] | ||
| #[cfg(not(any(Py_LIMITED_API, RustPython)))] |
There was a problem hiding this comment.
Are the changes here still needed given the change to pyo3-build-config?
There was a problem hiding this comment.
Not really but maybe a nice sanity check
fixes #6047
Tested using: