Add bindings for PyDict_SetDefaultRef and use them in PyCode::run#5955
Add bindings for PyDict_SetDefaultRef and use them in PyCode::run#5955
Conversation
ngoldbaum
force-pushed
the
globals-set-default
branch
from
69e4dab to
13a0dc4
Compare
|
It looks like we don't have extensive coverage tests for our compat shims. Maybe we should? I'd prefer not to add infrastructure for that in this PR though. |
davidhewitt
left a comment
davidhewitt
left a comment
There was a problem hiding this comment.
Thanks, one thought on the usage of this.
Indeed we don't have full coverage of the compat shims, generally coverage of code in pyo3-ffi is quite low because it's hard to test the FFI code without the convenience layers in pyo3.
We have some tests in src/ffi/tests inside the pyo3 lib, we should probably expand those (maybe warrants an issue?).
For this particular case, maybe we should add setdefault to PyDictMethods? (We could then use that in PyCode::run for bonus safety.)
| // safety: the interpreter will keep the borrowed reference to | ||
| // builtins alive at least until SetDefaultRef finishes | ||
| ffi::PyEval_GetBuiltins(), | ||
| &mut result, |
There was a problem hiding this comment.
I think it's legal to set std::ptr::null_mut() directly here, which avoids the need for the decref below also?
2 participants
While experimenting with a build of PyO3 that supports PEP 803 without using critical sections, I noticed that the use in
PyCode::runcan be replaced byPyDict_SetDefaultRef. This also makes the implementation a lot simpler.I added bindings for that function on 3.13 and newer and 3.15 and newer in the limited API. I also added a compat shim based on the implementation in pythoncapi-compat: https://github.com/python/pythoncapi-compat/blob/75a796764d63327268d195e2d5c044e564d0dada/pythoncapi_compat.h#L1314.