From ded4f34675372fb0828c081ec3e3a3d9e1757471 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 16 Jul 2024 15:36:08 -0600 Subject: [PATCH 01/18] Update dict.get_item binding to use PyDict_GetItemRef Refs #4265 --- newsfragments/4355.fixed.md | 2 ++ pyo3-ffi/src/dictobject.rs | 26 ++++++++++++++++++++++++++ src/types/dict.rs | 13 ++++++------- 3 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 newsfragments/4355.fixed.md diff --git a/newsfragments/4355.fixed.md b/newsfragments/4355.fixed.md new file mode 100644 index 00000000000..9a141bc6b96 --- /dev/null +++ b/newsfragments/4355.fixed.md @@ -0,0 +1,2 @@ +Avoid creating temporary borrowed reference in dict.get_item bindings. Borrowed +references like this are unsafe in the free-threading build. diff --git a/pyo3-ffi/src/dictobject.rs b/pyo3-ffi/src/dictobject.rs index 99fc56b246b..9a5c4abb4d2 100644 --- a/pyo3-ffi/src/dictobject.rs +++ b/pyo3-ffi/src/dictobject.rs @@ -1,4 +1,6 @@ use crate::object::*; +#[cfg(not(Py_3_13))] +use crate::pyerrors::PyErr_Occurred; use crate::pyport::Py_ssize_t; use std::os::raw::{c_char, c_int}; use std::ptr::addr_of_mut; @@ -58,6 +60,12 @@ extern "C" { pub fn PyDict_MergeFromSeq2(d: *mut PyObject, seq2: *mut PyObject, _override: c_int) -> c_int; #[cfg_attr(PyPy, link_name = "PyPyDict_GetItemString")] pub fn PyDict_GetItemString(dp: *mut PyObject, key: *const c_char) -> *mut PyObject; + #[cfg(Py_3_13)] + pub fn PyDict_GetItemRef( + dp: *mut PyObject, + key: *mut PyObject, + result: *mut *mut PyObject, + ) -> c_int; #[cfg_attr(PyPy, link_name = "PyPyDict_SetItemString")] pub fn PyDict_SetItemString( dp: *mut PyObject, @@ -96,6 +104,24 @@ pub unsafe fn PyDictViewSet_Check(op: *mut PyObject) -> c_int { (PyDictKeys_Check(op) != 0 || PyDictItems_Check(op) != 0) as c_int } +#[cfg(not(Py_3_13))] +pub unsafe fn PyDict_GetItemRef( + dp: *mut PyObject, + key: *mut PyObject, + result: *mut *mut PyObject, +) -> c_int { + let item: *mut PyObject = PyDict_GetItemWithError(dp, key); + if !item.is_null() { + *result = _Py_NewRef(item); + return 1; // found + } + *result = std::ptr::null_mut(); + if !PyErr_Occurred().is_null() { + return 0; // not found + } + -1 +} + #[cfg_attr(windows, link(name = "pythonXY"))] extern "C" { pub static mut PyDictIterKey_Type: PyTypeObject; diff --git a/src/types/dict.rs b/src/types/dict.rs index 8231e4df94f..0f77f45dc52 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -247,13 +247,12 @@ impl<'py> PyDictMethods<'py> for Bound<'py, PyDict> { key: Bound<'_, PyAny>, ) -> PyResult>> { let py = dict.py(); - match unsafe { - ffi::PyDict_GetItemWithError(dict.as_ptr(), key.as_ptr()) - .assume_borrowed_or_opt(py) - .map(Borrowed::to_owned) - } { - some @ Some(_) => Ok(some), - None => PyErr::take(py).map(Err).transpose(), + unsafe { + let mut result: *mut ffi::PyObject = std::ptr::null_mut(); + match ffi::PyDict_GetItemRef(dict.as_ptr(), key.as_ptr(), &mut result) { + std::os::raw::c_int::MIN..=0 => PyErr::take(py).map(Err).transpose(), + 1..=std::os::raw::c_int::MAX => Ok(result.assume_owned_or_opt(py)), + } } } From 12495df020d45b12184b38b50adfb14c4884e873 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 23 Jul 2024 11:42:07 -0600 Subject: [PATCH 02/18] test: add test for dict.get_item error path --- src/types/dict.rs | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/types/dict.rs b/src/types/dict.rs index 0f77f45dc52..be6d0ffefd6 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -599,6 +599,7 @@ where #[cfg(test)] mod tests { use super::*; + use crate::exceptions::PyTypeError; use crate::types::PyTuple; use std::collections::{BTreeMap, HashMap}; @@ -726,6 +727,40 @@ mod tests { }); } + #[cfg(feature = "macros")] + #[test] + fn test_get_item_error_path() { + #[crate::pyclass(crate = "crate")] + struct HashErrors; + + #[crate::pymethods(crate = "crate")] + impl HashErrors { + #[new] + fn new() -> PyResult { + Ok(HashErrors {}) + } + + fn __hash__(&self) -> PyResult { + Err(PyTypeError::new_err("Error from __hash__")) + } + } + + Python::with_gil(|py| { + let class = py.get_type_bound::(); + let instance = class.call0().unwrap(); + let d = PyDict::new_bound(py); + match d.get_item(instance) { + Ok(_) => { + panic!("this get_item call should always error") + } + Err(err) => { + assert!(err.is_instance_of::(py)); + assert_eq!(err.value_bound(py).to_string(), "Error from __hash__") + } + } + }) + } + #[test] fn test_set_item() { Python::with_gil(|py| { From 09f87a131a31c499c7758e9355ad12390bcd0705 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 23 Jul 2024 12:35:37 -0600 Subject: [PATCH 03/18] test: add test for dict.get_item error path --- src/types/dict.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/types/dict.rs b/src/types/dict.rs index be6d0ffefd6..c8e5bcd74d9 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -735,11 +735,6 @@ mod tests { #[crate::pymethods(crate = "crate")] impl HashErrors { - #[new] - fn new() -> PyResult { - Ok(HashErrors {}) - } - fn __hash__(&self) -> PyResult { Err(PyTypeError::new_err("Error from __hash__")) } From 1ad7d406b7111fe3579cba541659c00a093969b0 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 23 Jul 2024 12:35:57 -0600 Subject: [PATCH 04/18] test: add test for dict.get_item error path --- src/types/dict.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/types/dict.rs b/src/types/dict.rs index c8e5bcd74d9..be6d0ffefd6 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -735,6 +735,11 @@ mod tests { #[crate::pymethods(crate = "crate")] impl HashErrors { + #[new] + fn new() -> PyResult { + Ok(HashErrors {}) + } + fn __hash__(&self) -> PyResult { Err(PyTypeError::new_err("Error from __hash__")) } From c82ccf09b7a8cc6d320605fa7530b24f806defd4 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 23 Jul 2024 13:17:26 -0600 Subject: [PATCH 05/18] fix: fix logic error in dict.get_item bindings --- pyo3-ffi/src/dictobject.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyo3-ffi/src/dictobject.rs b/pyo3-ffi/src/dictobject.rs index 9a5c4abb4d2..86af9ec9f04 100644 --- a/pyo3-ffi/src/dictobject.rs +++ b/pyo3-ffi/src/dictobject.rs @@ -116,7 +116,7 @@ pub unsafe fn PyDict_GetItemRef( return 1; // found } *result = std::ptr::null_mut(); - if !PyErr_Occurred().is_null() { + if PyErr_Occurred().is_null() { return 0; // not found } -1 From 243385df68efb6beec657b66140334c8dccef1e0 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 23 Jul 2024 13:17:49 -0600 Subject: [PATCH 06/18] update: apply david's review suggestions for dict.get_item bindings --- src/types/dict.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/types/dict.rs b/src/types/dict.rs index be6d0ffefd6..d6db5ba8e09 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -247,12 +247,11 @@ impl<'py> PyDictMethods<'py> for Bound<'py, PyDict> { key: Bound<'_, PyAny>, ) -> PyResult>> { let py = dict.py(); - unsafe { - let mut result: *mut ffi::PyObject = std::ptr::null_mut(); - match ffi::PyDict_GetItemRef(dict.as_ptr(), key.as_ptr(), &mut result) { - std::os::raw::c_int::MIN..=0 => PyErr::take(py).map(Err).transpose(), - 1..=std::os::raw::c_int::MAX => Ok(result.assume_owned_or_opt(py)), - } + let mut result: *mut ffi::PyObject = std::ptr::null_mut(); + match unsafe { ffi::PyDict_GetItemRef(dict.as_ptr(), key.as_ptr(), &mut result) } { + std::os::raw::c_int::MIN..=-1 => Err(PyErr::fetch(py)), + 0 => Ok(None), + 1..=std::os::raw::c_int::MAX => Ok(Some(unsafe { result.assume_owned(py) })), } } From cb66f2f913a2ec7ad6a2f8d3e044a2513f9bf122 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 23 Jul 2024 13:54:15 -0600 Subject: [PATCH 07/18] update: create ffi::compat to store compatibility shims --- pyo3-ffi/src/compat.rs | 42 ++++++++++++++++++++++++++++++++++++++ pyo3-ffi/src/dictobject.rs | 20 ------------------ pyo3-ffi/src/lib.rs | 1 + src/types/dict.rs | 4 +++- 4 files changed, 46 insertions(+), 21 deletions(-) create mode 100644 pyo3-ffi/src/compat.rs diff --git a/pyo3-ffi/src/compat.rs b/pyo3-ffi/src/compat.rs new file mode 100644 index 00000000000..6a332f602eb --- /dev/null +++ b/pyo3-ffi/src/compat.rs @@ -0,0 +1,42 @@ +//! C API Compatibility Shims +//! +//! Some CPython C API functions added in recent versions of Python are +//! inherently safer to use than older C API constructs. This module +//! exposes versions of these safer functions for older python versions +//! and on newer versions simply re-exports the function from the FFI +//! bindings. +//! +//! This compatibility module makes it easier to use safer C API +//! constructs without writing your own compatibility shims. + +#[cfg(Py_3_13)] +mod py313_compat { + use crate::dictobject::PyDict_GetItemRef; +} +#[cfg(not(Py_3_13))] +mod py313_compat { + use crate::object::{PyObject, _Py_NewRef}; + use std::os::raw::c_int; + + use crate::dictobject::PyDict_GetItemWithError; + use crate::pyerrors::PyErr_Occurred; + + pub unsafe fn PyDict_GetItemRef( + dp: *mut PyObject, + key: *mut PyObject, + result: *mut *mut PyObject, + ) -> c_int { + let item: *mut PyObject = PyDict_GetItemWithError(dp, key); + if !item.is_null() { + *result = _Py_NewRef(item); + return 1; // found + } + *result = std::ptr::null_mut(); + if PyErr_Occurred().is_null() { + return 0; // not found + } + -1 + } +} + +pub use py313_compat::PyDict_GetItemRef; diff --git a/pyo3-ffi/src/dictobject.rs b/pyo3-ffi/src/dictobject.rs index 86af9ec9f04..346a2e4ce57 100644 --- a/pyo3-ffi/src/dictobject.rs +++ b/pyo3-ffi/src/dictobject.rs @@ -1,6 +1,4 @@ use crate::object::*; -#[cfg(not(Py_3_13))] -use crate::pyerrors::PyErr_Occurred; use crate::pyport::Py_ssize_t; use std::os::raw::{c_char, c_int}; use std::ptr::addr_of_mut; @@ -104,24 +102,6 @@ pub unsafe fn PyDictViewSet_Check(op: *mut PyObject) -> c_int { (PyDictKeys_Check(op) != 0 || PyDictItems_Check(op) != 0) as c_int } -#[cfg(not(Py_3_13))] -pub unsafe fn PyDict_GetItemRef( - dp: *mut PyObject, - key: *mut PyObject, - result: *mut *mut PyObject, -) -> c_int { - let item: *mut PyObject = PyDict_GetItemWithError(dp, key); - if !item.is_null() { - *result = _Py_NewRef(item); - return 1; // found - } - *result = std::ptr::null_mut(); - if PyErr_Occurred().is_null() { - return 0; // not found - } - -1 -} - #[cfg_attr(windows, link(name = "pythonXY"))] extern "C" { pub static mut PyDictIterKey_Type: PyTypeObject; diff --git a/pyo3-ffi/src/lib.rs b/pyo3-ffi/src/lib.rs index ff4d03d3a44..b0b4b42f637 100644 --- a/pyo3-ffi/src/lib.rs +++ b/pyo3-ffi/src/lib.rs @@ -364,6 +364,7 @@ mod ceval; #[cfg(Py_LIMITED_API)] mod code; mod codecs; +pub mod compat; mod compile; mod complexobject; #[cfg(all(Py_3_8, not(Py_LIMITED_API)))] diff --git a/src/types/dict.rs b/src/types/dict.rs index d6db5ba8e09..a9b3e23b54c 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -248,7 +248,9 @@ impl<'py> PyDictMethods<'py> for Bound<'py, PyDict> { ) -> PyResult>> { let py = dict.py(); let mut result: *mut ffi::PyObject = std::ptr::null_mut(); - match unsafe { ffi::PyDict_GetItemRef(dict.as_ptr(), key.as_ptr(), &mut result) } { + match unsafe { + ffi::compat::PyDict_GetItemRef(dict.as_ptr(), key.as_ptr(), &mut result) + } { std::os::raw::c_int::MIN..=-1 => Err(PyErr::fetch(py)), 0 => Ok(None), 1..=std::os::raw::c_int::MAX => Ok(Some(unsafe { result.assume_owned(py) })), From ac5394dfe2ff818333982b0bae9fe988780b6bc8 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 23 Jul 2024 13:56:38 -0600 Subject: [PATCH 08/18] update: move PyDict_GetItemRef bindings to spot in order from dictobject.h --- pyo3-ffi/src/dictobject.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pyo3-ffi/src/dictobject.rs b/pyo3-ffi/src/dictobject.rs index 346a2e4ce57..4d8315d441e 100644 --- a/pyo3-ffi/src/dictobject.rs +++ b/pyo3-ffi/src/dictobject.rs @@ -58,12 +58,6 @@ extern "C" { pub fn PyDict_MergeFromSeq2(d: *mut PyObject, seq2: *mut PyObject, _override: c_int) -> c_int; #[cfg_attr(PyPy, link_name = "PyPyDict_GetItemString")] pub fn PyDict_GetItemString(dp: *mut PyObject, key: *const c_char) -> *mut PyObject; - #[cfg(Py_3_13)] - pub fn PyDict_GetItemRef( - dp: *mut PyObject, - key: *mut PyObject, - result: *mut *mut PyObject, - ) -> c_int; #[cfg_attr(PyPy, link_name = "PyPyDict_SetItemString")] pub fn PyDict_SetItemString( dp: *mut PyObject, @@ -72,6 +66,12 @@ extern "C" { ) -> c_int; #[cfg_attr(PyPy, link_name = "PyPyDict_DelItemString")] pub fn PyDict_DelItemString(dp: *mut PyObject, key: *const c_char) -> c_int; + #[cfg(Py_3_13)] + pub fn PyDict_GetItemRef( + dp: *mut PyObject, + key: *mut PyObject, + result: *mut *mut PyObject, + ) -> c_int; // skipped 3.10 / ex-non-limited PyObject_GenericGetDict } From ebf96021650b8e2ae20fc042c2ad3f25861cfd85 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 23 Jul 2024 14:09:09 -0600 Subject: [PATCH 09/18] build: fix build warning with --no-default-features --- src/types/dict.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/types/dict.rs b/src/types/dict.rs index a9b3e23b54c..aa223f6e719 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -600,7 +600,6 @@ where #[cfg(test)] mod tests { use super::*; - use crate::exceptions::PyTypeError; use crate::types::PyTuple; use std::collections::{BTreeMap, HashMap}; @@ -731,6 +730,8 @@ mod tests { #[cfg(feature = "macros")] #[test] fn test_get_item_error_path() { + use crate::exceptions::PyTypeError; + #[crate::pyclass(crate = "crate")] struct HashErrors; From 49ea2fcb312ab1ff02f47d73f6191e4544ec857a Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 23 Jul 2024 14:14:45 -0600 Subject: [PATCH 10/18] doc: expand release note fragments --- newsfragments/4355.added.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 newsfragments/4355.added.md diff --git a/newsfragments/4355.added.md b/newsfragments/4355.added.md new file mode 100644 index 00000000000..1410c0720bf --- /dev/null +++ b/newsfragments/4355.added.md @@ -0,0 +1,10 @@ +* Added an `ffi::compat` namespace to store compatibility shims for C API + functions added in recent versions of Python. + +* Added bindings for `PyDict_GetItemRef` on Python 3.13 and newer. Also added + `ffi::compat::PyDict_GetItemRef` which re-exports the FFI binding on Python + 3.13 or newer and defines a compatibility version on older versions of + Python. This function is inherently safer to use than `PyDict_GetItem` and has + an API that is easier to use than `PyDict_GetItemWithError`. It returns a + strong reference to value, as opposed to the two older functions which return + a possibly unsafe borrowed reference. From a2e3595fabeade074a96089b9c4ae8f206df9cc9 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 23 Jul 2024 14:26:24 -0600 Subject: [PATCH 11/18] fix: fix clippy warnings --- pyo3-ffi/src/compat.rs | 2 +- src/types/dict.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pyo3-ffi/src/compat.rs b/pyo3-ffi/src/compat.rs index 6a332f602eb..aadfb7b6670 100644 --- a/pyo3-ffi/src/compat.rs +++ b/pyo3-ffi/src/compat.rs @@ -11,7 +11,7 @@ #[cfg(Py_3_13)] mod py313_compat { - use crate::dictobject::PyDict_GetItemRef; + pub use crate::dictobject::PyDict_GetItemRef; } #[cfg(not(Py_3_13))] mod py313_compat { diff --git a/src/types/dict.rs b/src/types/dict.rs index aa223f6e719..d9650614723 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -738,8 +738,8 @@ mod tests { #[crate::pymethods(crate = "crate")] impl HashErrors { #[new] - fn new() -> PyResult { - Ok(HashErrors {}) + fn new() -> Self { + HashErrors {} } fn __hash__(&self) -> PyResult { From 3fe8d0be01cecfb9a84840fa78aab0aed169ce8f Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 26 Jul 2024 13:34:19 -0600 Subject: [PATCH 12/18] respond to review comments --- pyo3-ffi/src/compat.rs | 44 ++++++++++++++++++++---------------------- pyo3-ffi/src/lib.rs | 3 ++- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/pyo3-ffi/src/compat.rs b/pyo3-ffi/src/compat.rs index aadfb7b6670..61a754b037d 100644 --- a/pyo3-ffi/src/compat.rs +++ b/pyo3-ffi/src/compat.rs @@ -2,30 +2,30 @@ //! //! Some CPython C API functions added in recent versions of Python are //! inherently safer to use than older C API constructs. This module -//! exposes versions of these safer functions for older python versions -//! and on newer versions simply re-exports the function from the FFI -//! bindings. -//! -//! This compatibility module makes it easier to use safer C API -//! constructs without writing your own compatibility shims. +//! exposes functions available on all Python versions that wrap the +//! old C API on old Python versions and wrap the function directly +//! on newer Python versions. -#[cfg(Py_3_13)] -mod py313_compat { - pub use crate::dictobject::PyDict_GetItemRef; -} -#[cfg(not(Py_3_13))] -mod py313_compat { - use crate::object::{PyObject, _Py_NewRef}; - use std::os::raw::c_int; +use crate::object::PyObject; +use std::os::raw::c_int; - use crate::dictobject::PyDict_GetItemWithError; - use crate::pyerrors::PyErr_Occurred; +pub unsafe fn PyDict_GetItemRef( + dp: *mut PyObject, + key: *mut PyObject, + result: *mut *mut PyObject, +) -> c_int { + #[cfg(Py_3_13)] + { + crate::PyDict_GetItemRef(dp, key, result) + } - pub unsafe fn PyDict_GetItemRef( - dp: *mut PyObject, - key: *mut PyObject, - result: *mut *mut PyObject, - ) -> c_int { + #[cfg(not(Py_3_13))] + { + use crate::dictobject::PyDict_GetItemWithError; + use crate::object::_Py_NewRef; + use crate::pyerrors::PyErr_Occurred; + + // adapted from pythoncapi-compat let item: *mut PyObject = PyDict_GetItemWithError(dp, key); if !item.is_null() { *result = _Py_NewRef(item); @@ -38,5 +38,3 @@ mod py313_compat { -1 } } - -pub use py313_compat::PyDict_GetItemRef; diff --git a/pyo3-ffi/src/lib.rs b/pyo3-ffi/src/lib.rs index b0b4b42f637..fb4ee46fd39 100644 --- a/pyo3-ffi/src/lib.rs +++ b/pyo3-ffi/src/lib.rs @@ -290,6 +290,8 @@ pub const fn _cstr_from_utf8_with_nul_checked(s: &str) -> &CStr { use std::ffi::CStr; +pub mod compat; + pub use self::abstract_::*; pub use self::bltinmodule::*; pub use self::boolobject::*; @@ -364,7 +366,6 @@ mod ceval; #[cfg(Py_LIMITED_API)] mod code; mod codecs; -pub mod compat; mod compile; mod complexobject; #[cfg(all(Py_3_8, not(Py_LIMITED_API)))] From f3df5feb111a741a7c72f69201816efdf2badb53 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 29 Jul 2024 14:20:08 -0600 Subject: [PATCH 13/18] Apply suggestion from @mejrs --- pyo3-ffi/src/compat.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pyo3-ffi/src/compat.rs b/pyo3-ffi/src/compat.rs index 61a754b037d..1711208c19a 100644 --- a/pyo3-ffi/src/compat.rs +++ b/pyo3-ffi/src/compat.rs @@ -1,3 +1,5 @@ +#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] + //! C API Compatibility Shims //! //! Some CPython C API functions added in recent versions of Python are @@ -6,6 +8,9 @@ //! old C API on old Python versions and wrap the function directly //! on newer Python versions. +// Unless otherwise noted, the compatibility shims are adapted from +// the pythoncapi-compat project: https://github.com/python/pythoncapi-compat + use crate::object::PyObject; use std::os::raw::c_int; @@ -14,18 +19,19 @@ pub unsafe fn PyDict_GetItemRef( key: *mut PyObject, result: *mut *mut PyObject, ) -> c_int { + #[cfg_attr(docsrs, doc(cfg()))] #[cfg(Py_3_13)] { crate::PyDict_GetItemRef(dp, key, result) } + #[cfg_attr(docsrs, doc(cfg()))] #[cfg(not(Py_3_13))] { use crate::dictobject::PyDict_GetItemWithError; use crate::object::_Py_NewRef; use crate::pyerrors::PyErr_Occurred; - // adapted from pythoncapi-compat let item: *mut PyObject = PyDict_GetItemWithError(dp, key); if !item.is_null() { *result = _Py_NewRef(item); From c7bda5198d1fccdb1052419deade618f5591a7b9 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 29 Jul 2024 15:37:23 -0600 Subject: [PATCH 14/18] refactor so cfg is applied to functions --- pyo3-ffi/src/compat.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/pyo3-ffi/src/compat.rs b/pyo3-ffi/src/compat.rs index 1711208c19a..7e53b55e299 100644 --- a/pyo3-ffi/src/compat.rs +++ b/pyo3-ffi/src/compat.rs @@ -14,19 +14,17 @@ use crate::object::PyObject; use std::os::raw::c_int; +#[cfg_attr(docsrs, doc(cfg()))] +#[cfg(Py_3_13)] +pub use crate::dictobject::PyDict_GetItemRef; + +#[cfg_attr(docsrs, doc(cfg()))] +#[cfg(not(Py_3_13))] pub unsafe fn PyDict_GetItemRef( dp: *mut PyObject, key: *mut PyObject, result: *mut *mut PyObject, ) -> c_int { - #[cfg_attr(docsrs, doc(cfg()))] - #[cfg(Py_3_13)] - { - crate::PyDict_GetItemRef(dp, key, result) - } - - #[cfg_attr(docsrs, doc(cfg()))] - #[cfg(not(Py_3_13))] { use crate::dictobject::PyDict_GetItemWithError; use crate::object::_Py_NewRef; From 9b7cf0f4ade6c3d204c02e499669cc51265d9204 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 29 Jul 2024 15:42:14 -0600 Subject: [PATCH 15/18] properly set cfgs --- pyo3-ffi/src/compat.rs | 6 ++---- pyo3-ffi/src/lib.rs | 1 + 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pyo3-ffi/src/compat.rs b/pyo3-ffi/src/compat.rs index 7e53b55e299..6dd27b6f67a 100644 --- a/pyo3-ffi/src/compat.rs +++ b/pyo3-ffi/src/compat.rs @@ -1,5 +1,3 @@ -#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] - //! C API Compatibility Shims //! //! Some CPython C API functions added in recent versions of Python are @@ -14,11 +12,11 @@ use crate::object::PyObject; use std::os::raw::c_int; -#[cfg_attr(docsrs, doc(cfg()))] +#[cfg_attr(docsrs, doc(cfg(Py_3_13)))] #[cfg(Py_3_13)] pub use crate::dictobject::PyDict_GetItemRef; -#[cfg_attr(docsrs, doc(cfg()))] +#[cfg_attr(docsrs, doc(cfg(not(Py_3_13))))] #[cfg(not(Py_3_13))] pub unsafe fn PyDict_GetItemRef( dp: *mut PyObject, diff --git a/pyo3-ffi/src/lib.rs b/pyo3-ffi/src/lib.rs index fb4ee46fd39..55c7f31404f 100644 --- a/pyo3-ffi/src/lib.rs +++ b/pyo3-ffi/src/lib.rs @@ -1,3 +1,4 @@ +#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] //! Raw FFI declarations for Python's C API. //! //! PyO3 can be used to write native Python modules or run Python code and modules from Rust. From 55c760384dff352f67f231cb48e1c29578024f72 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 29 Jul 2024 16:41:26 -0600 Subject: [PATCH 16/18] fix clippy lints --- pyo3-ffi/src/compat.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyo3-ffi/src/compat.rs b/pyo3-ffi/src/compat.rs index 6dd27b6f67a..591e3724c9d 100644 --- a/pyo3-ffi/src/compat.rs +++ b/pyo3-ffi/src/compat.rs @@ -9,7 +9,9 @@ // Unless otherwise noted, the compatibility shims are adapted from // the pythoncapi-compat project: https://github.com/python/pythoncapi-compat +#[cfg(not(Py_3_13))] use crate::object::PyObject; +#[cfg(not(Py_3_13))] use std::os::raw::c_int; #[cfg_attr(docsrs, doc(cfg(Py_3_13)))] From 963f6c28af9501f501cbc32976b02bd30e7b161d Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 30 Jul 2024 08:57:09 -0600 Subject: [PATCH 17/18] Apply @davidhewitt's suggestion --- pyo3-ffi/src/compat.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyo3-ffi/src/compat.rs b/pyo3-ffi/src/compat.rs index 591e3724c9d..ef9a3fbe1c9 100644 --- a/pyo3-ffi/src/compat.rs +++ b/pyo3-ffi/src/compat.rs @@ -14,11 +14,11 @@ use crate::object::PyObject; #[cfg(not(Py_3_13))] use std::os::raw::c_int; -#[cfg_attr(docsrs, doc(cfg(Py_3_13)))] +#[cfg_attr(docsrs, doc(cfg(all)))] #[cfg(Py_3_13)] pub use crate::dictobject::PyDict_GetItemRef; -#[cfg_attr(docsrs, doc(cfg(not(Py_3_13))))] +#[cfg_attr(docsrs, doc(cfg(all)))] #[cfg(not(Py_3_13))] pub unsafe fn PyDict_GetItemRef( dp: *mut PyObject, From 44d1f9e51aff6e8403937a60217fa67d036e6cdb Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 1 Aug 2024 09:16:34 -0600 Subject: [PATCH 18/18] deal with upstream deprecation of new_bound --- src/types/dict.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types/dict.rs b/src/types/dict.rs index d9650614723..20664541f0c 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -750,7 +750,7 @@ mod tests { Python::with_gil(|py| { let class = py.get_type_bound::(); let instance = class.call0().unwrap(); - let d = PyDict::new_bound(py); + let d = PyDict::new(py); match d.get_item(instance) { Ok(_) => { panic!("this get_item call should always error")