From a3d13234f43f82719dad8fb3049fd2a220fa469d Mon Sep 17 00:00:00 2001
From: David Hewitt <mail@davidhewitt.dev>
Date: Fri, 16 Aug 2024 15:27:19 +0100
Subject: [PATCH 1/5] ffi: define compat for `Py_NewRef` and `Py_XNewRef`

---
 newsfragments/4445.added.md       |  1 +
 newsfragments/4445.removed.md     |  1 +
 pyo3-ffi/src/compat.rs            | 60 -------------------------------
 pyo3-ffi/src/compat/mod.rs        | 39 ++++++++++++++++++++
 pyo3-ffi/src/compat/py_3_10.rs    | 18 ++++++++++
 pyo3-ffi/src/compat/py_3_13.rs    | 39 ++++++++++++++++++++
 pyo3-ffi/src/object.rs            | 27 ++++++--------
 src/pyclass/create_type_object.rs |  2 +-
 8 files changed, 109 insertions(+), 78 deletions(-)
 create mode 100644 newsfragments/4445.added.md
 create mode 100644 newsfragments/4445.removed.md
 delete mode 100644 pyo3-ffi/src/compat.rs
 create mode 100644 pyo3-ffi/src/compat/mod.rs
 create mode 100644 pyo3-ffi/src/compat/py_3_10.rs
 create mode 100644 pyo3-ffi/src/compat/py_3_13.rs

diff --git a/newsfragments/4445.added.md b/newsfragments/4445.added.md
new file mode 100644
index 00000000000..ee6af52d97e
--- /dev/null
+++ b/newsfragments/4445.added.md
@@ -0,0 +1 @@
+Add FFI definitions `compat::Py_NewRef` and `compat::Py_XNewRef`.
diff --git a/newsfragments/4445.removed.md b/newsfragments/4445.removed.md
new file mode 100644
index 00000000000..b5b7e450331
--- /dev/null
+++ b/newsfragments/4445.removed.md
@@ -0,0 +1 @@
+Remove private FFI definitions `_Py_NewRef` and `_Py_XNewRef`.
diff --git a/pyo3-ffi/src/compat.rs b/pyo3-ffi/src/compat.rs
deleted file mode 100644
index 85986817d93..00000000000
--- a/pyo3-ffi/src/compat.rs
+++ /dev/null
@@ -1,60 +0,0 @@
-//! 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 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.
-
-// 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 crate::pyport::Py_ssize_t;
-#[cfg(not(Py_3_13))]
-use std::os::raw::c_int;
-
-#[cfg_attr(docsrs, doc(cfg(all())))]
-#[cfg(Py_3_13)]
-pub use crate::dictobject::PyDict_GetItemRef;
-
-#[cfg_attr(docsrs, doc(cfg(all())))]
-#[cfg(not(Py_3_13))]
-pub unsafe fn PyDict_GetItemRef(
-    dp: *mut PyObject,
-    key: *mut PyObject,
-    result: *mut *mut PyObject,
-) -> c_int {
-    {
-        use crate::dictobject::PyDict_GetItemWithError;
-        use crate::object::_Py_NewRef;
-        use crate::pyerrors::PyErr_Occurred;
-
-        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(docsrs, doc(cfg(all())))]
-#[cfg(Py_3_13)]
-pub use crate::PyList_GetItemRef;
-
-#[cfg(not(Py_3_13))]
-#[cfg_attr(docsrs, doc(cfg(all())))]
-pub unsafe fn PyList_GetItemRef(arg1: *mut PyObject, arg2: Py_ssize_t) -> *mut PyObject {
-    use crate::{PyList_GetItem, Py_XINCREF};
-
-    let item: *mut PyObject = PyList_GetItem(arg1, arg2);
-    Py_XINCREF(item);
-    item
-}
diff --git a/pyo3-ffi/src/compat/mod.rs b/pyo3-ffi/src/compat/mod.rs
new file mode 100644
index 00000000000..fd8dc077001
--- /dev/null
+++ b/pyo3-ffi/src/compat/mod.rs
@@ -0,0 +1,39 @@
+//! 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 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.
+
+// Unless otherwise noted, the compatibility shims are adapted from
+// the pythoncapi-compat project: https://github.com/python/pythoncapi-compat
+
+/// Internal helper macro which defines compatibility shims for C API functions, deferring to a
+/// re-export when that's available.
+macro_rules! compat_function {
+    (
+        originally_defined_for($cfg:meta);
+
+        $(#[$attrs:meta])*
+        pub unsafe fn $name:ident($($arg_names:ident: $arg_types:ty),* $(,)?) -> $ret:ty $body:block
+    ) => {
+        // Define as a standalone function under docsrs cfg so that this shows as a unique function in the docs,
+        // not a re-export (the re-export has the wrong visibility)
+        #[cfg(any(docsrs, not($cfg)))]
+        #[cfg_attr(docsrs, doc(cfg(all())))]
+        $(#[$attrs])*
+        pub unsafe fn $name(
+            $($arg_names: $arg_types,)*
+        ) -> $ret $body
+
+        #[cfg(all($cfg, not(docsrs)))]
+        pub use $crate::$name;
+    };
+}
+
+mod py_3_10;
+mod py_3_13;
+
+pub use self::py_3_10::*;
+pub use self::py_3_13::*;
diff --git a/pyo3-ffi/src/compat/py_3_10.rs b/pyo3-ffi/src/compat/py_3_10.rs
new file mode 100644
index 00000000000..2a92fc3b1da
--- /dev/null
+++ b/pyo3-ffi/src/compat/py_3_10.rs
@@ -0,0 +1,18 @@
+compat_function!(
+    originally_defined_for(Py_3_10);
+
+    pub unsafe fn Py_NewRef(obj: *mut crate::PyObject) -> *mut crate::PyObject {
+        crate::Py_INCREF(obj);
+        obj
+    }
+);
+
+compat_function!(
+    originally_defined_for(Py_3_10);
+
+    #[inline]
+    pub unsafe fn Py_XNewRef(obj: *mut crate::PyObject) -> *mut crate::PyObject {
+        crate::Py_XINCREF(obj);
+        obj
+    }
+);
diff --git a/pyo3-ffi/src/compat/py_3_13.rs b/pyo3-ffi/src/compat/py_3_13.rs
new file mode 100644
index 00000000000..f771b853d77
--- /dev/null
+++ b/pyo3-ffi/src/compat/py_3_13.rs
@@ -0,0 +1,39 @@
+compat_function!(
+    originally_defined_for(Py_3_13);
+
+    #[inline]
+    pub unsafe fn PyDict_GetItemRef(
+        dp: *mut crate::PyObject,
+        key: *mut crate::PyObject,
+        result: *mut *mut crate::PyObject,
+    ) -> std::ffi::c_int {
+        use crate::{compat::Py_NewRef, PyDict_GetItemWithError, PyErr_Occurred};
+
+        let item = 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
+    }
+);
+
+compat_function!(
+    originally_defined_for(Py_3_13);
+
+    #[inline]
+    pub unsafe fn PyList_GetItemRef(
+        arg1: *mut crate::PyObject,
+        arg2: crate::Py_ssize_t,
+    ) -> *mut crate::PyObject {
+        use crate::{PyList_GetItem, Py_XINCREF};
+
+        let item = PyList_GetItem(arg1, arg2);
+        Py_XINCREF(item);
+        item
+    }
+);
diff --git a/pyo3-ffi/src/object.rs b/pyo3-ffi/src/object.rs
index 9181cb17dd2..4408ef38b5e 100644
--- a/pyo3-ffi/src/object.rs
+++ b/pyo3-ffi/src/object.rs
@@ -648,37 +648,30 @@ pub unsafe fn Py_XDECREF(op: *mut PyObject) {
 
 extern "C" {
     #[cfg(all(Py_3_10, Py_LIMITED_API))]
+    #[cfg_attr(docsrs, doc(cfg(Py_3_10)))]
     pub fn Py_NewRef(obj: *mut PyObject) -> *mut PyObject;
     #[cfg(all(Py_3_10, Py_LIMITED_API))]
+    #[cfg_attr(docsrs, doc(cfg(Py_3_10)))]
     pub fn Py_XNewRef(obj: *mut PyObject) -> *mut PyObject;
 }
 
-// Technically these macros are only available in the C header from 3.10 and up, however their
-// implementation works on all supported Python versions so we define these macros on all
-// versions for simplicity.
-
-#[inline]
-pub unsafe fn _Py_NewRef(obj: *mut PyObject) -> *mut PyObject {
-    Py_INCREF(obj);
-    obj
-}
-
-#[inline]
-pub unsafe fn _Py_XNewRef(obj: *mut PyObject) -> *mut PyObject {
-    Py_XINCREF(obj);
-    obj
-}
+// macro _Py_NewRef not public; reimplemented directly inside Py_NewRef here
+// macro _Py_XNewRef not public; reimplemented directly inside Py_XNewRef here
 
 #[cfg(all(Py_3_10, not(Py_LIMITED_API)))]
+#[cfg_attr(docsrs, doc(cfg(Py_3_10)))]
 #[inline]
 pub unsafe fn Py_NewRef(obj: *mut PyObject) -> *mut PyObject {
-    _Py_NewRef(obj)
+    Py_INCREF(obj);
+    obj
 }
 
 #[cfg(all(Py_3_10, not(Py_LIMITED_API)))]
+#[cfg_attr(docsrs, doc(cfg(Py_3_10)))]
 #[inline]
 pub unsafe fn Py_XNewRef(obj: *mut PyObject) -> *mut PyObject {
-    _Py_XNewRef(obj)
+    Py_XINCREF(obj);
+    obj
 }
 
 #[cfg_attr(windows, link(name = "pythonXY"))]
diff --git a/src/pyclass/create_type_object.rs b/src/pyclass/create_type_object.rs
index ac5b6287e86..b05d164e2b0 100644
--- a/src/pyclass/create_type_object.rs
+++ b/src/pyclass/create_type_object.rs
@@ -250,7 +250,7 @@ impl PyTypeBuilder {
                             if (*dict_ptr).is_null() {
                                 std::ptr::write(dict_ptr, ffi::PyDict_New());
                             }
-                            Ok(ffi::_Py_XNewRef(*dict_ptr))
+                            Ok(ffi::compat::Py_XNewRef(*dict_ptr))
                         })
                     }
                 }

From 9db87b912d4cfea7bd5378720c4e5d64858b18a5 Mon Sep 17 00:00:00 2001
From: David Hewitt <mail@davidhewitt.dev>
Date: Fri, 16 Aug 2024 21:19:34 +0100
Subject: [PATCH 2/5] add missing inline hint

Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
---
 pyo3-ffi/src/compat/py_3_10.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pyo3-ffi/src/compat/py_3_10.rs b/pyo3-ffi/src/compat/py_3_10.rs
index 2a92fc3b1da..463131e2e1d 100644
--- a/pyo3-ffi/src/compat/py_3_10.rs
+++ b/pyo3-ffi/src/compat/py_3_10.rs
@@ -1,6 +1,7 @@
 compat_function!(
     originally_defined_for(Py_3_10);
 
+#[inline]
     pub unsafe fn Py_NewRef(obj: *mut crate::PyObject) -> *mut crate::PyObject {
         crate::Py_INCREF(obj);
         obj

From 4080bb3680eabb3e0b8ab6e6c866c125482afc49 Mon Sep 17 00:00:00 2001
From: David Hewitt <mail@davidhewitt.dev>
Date: Fri, 16 Aug 2024 21:39:12 +0100
Subject: [PATCH 3/5] don't use std::ffi::c_int (requires MSRV 1.64)

---
 pyo3-ffi/src/compat/py_3_13.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pyo3-ffi/src/compat/py_3_13.rs b/pyo3-ffi/src/compat/py_3_13.rs
index f771b853d77..75c4cd101ae 100644
--- a/pyo3-ffi/src/compat/py_3_13.rs
+++ b/pyo3-ffi/src/compat/py_3_13.rs
@@ -6,7 +6,7 @@ compat_function!(
         dp: *mut crate::PyObject,
         key: *mut crate::PyObject,
         result: *mut *mut crate::PyObject,
-    ) -> std::ffi::c_int {
+    ) -> std::os::raw::c_int {
         use crate::{compat::Py_NewRef, PyDict_GetItemWithError, PyErr_Occurred};
 
         let item = PyDict_GetItemWithError(dp, key);

From 4d7c8ff48a00f8e5c7aae911eb650fbd017a006e Mon Sep 17 00:00:00 2001
From: David Hewitt <mail@davidhewitt.dev>
Date: Fri, 16 Aug 2024 23:01:36 +0100
Subject: [PATCH 4/5] add test to guard against ambiguity

---
 pyo3-ffi/Cargo.toml            |  3 +++
 pyo3-ffi/src/compat/mod.rs     | 18 ++++++++++++++++++
 pyo3-ffi/src/compat/py_3_10.rs |  2 +-
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/pyo3-ffi/Cargo.toml b/pyo3-ffi/Cargo.toml
index 8e1b203fcd1..0e58197bbfd 100644
--- a/pyo3-ffi/Cargo.toml
+++ b/pyo3-ffi/Cargo.toml
@@ -37,6 +37,9 @@ abi3-py312 = ["abi3", "pyo3-build-config/abi3-py312"]
 # Automatically generates `python3.dll` import libraries for Windows targets.
 generate-import-lib = ["pyo3-build-config/python3-dll-a"]
 
+[dev-dependencies]
+paste = "1"
+
 [build-dependencies]
 pyo3-build-config = { path = "../pyo3-build-config", version = "=0.23.0-dev", features = ["resolve-config"] }
 
diff --git a/pyo3-ffi/src/compat/mod.rs b/pyo3-ffi/src/compat/mod.rs
index fd8dc077001..db41c834b3d 100644
--- a/pyo3-ffi/src/compat/mod.rs
+++ b/pyo3-ffi/src/compat/mod.rs
@@ -29,6 +29,24 @@ macro_rules! compat_function {
 
         #[cfg(all($cfg, not(docsrs)))]
         pub use $crate::$name;
+
+        #[cfg(test)]
+        paste::paste! {
+            // Test that the compat function does not overlap with the original function. If the
+            // cfgs line up, then the the two glob imports will resolve to the same item via the
+            // re-export. If the cfgs mismatch, then the use of $name will be ambiguous in cases
+            // where the function is defined twice, and the test will fail to compile.
+            #[allow(unused_imports)]
+            mod [<test_ $name _export>] {
+                use $crate::*;
+                use $crate::compat::*;
+
+                #[test]
+                fn test_export() {
+                    let _ = $name;
+                }
+            }
+        }
     };
 }
 
diff --git a/pyo3-ffi/src/compat/py_3_10.rs b/pyo3-ffi/src/compat/py_3_10.rs
index 463131e2e1d..c6e8c2cb5ca 100644
--- a/pyo3-ffi/src/compat/py_3_10.rs
+++ b/pyo3-ffi/src/compat/py_3_10.rs
@@ -1,7 +1,7 @@
 compat_function!(
     originally_defined_for(Py_3_10);
 
-#[inline]
+    #[inline]
     pub unsafe fn Py_NewRef(obj: *mut crate::PyObject) -> *mut crate::PyObject {
         crate::Py_INCREF(obj);
         obj

From cd7e53ebfa11be00b159047bd079c70dfc552625 Mon Sep 17 00:00:00 2001
From: David Hewitt <mail@davidhewitt.dev>
Date: Sat, 17 Aug 2024 08:03:52 +0100
Subject: [PATCH 5/5] fix `Py_NewRef` cfg on PyPy

---
 pyo3-ffi/src/object.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/pyo3-ffi/src/object.rs b/pyo3-ffi/src/object.rs
index 4408ef38b5e..73b07ec2cf2 100644
--- a/pyo3-ffi/src/object.rs
+++ b/pyo3-ffi/src/object.rs
@@ -647,10 +647,10 @@ pub unsafe fn Py_XDECREF(op: *mut PyObject) {
 }
 
 extern "C" {
-    #[cfg(all(Py_3_10, Py_LIMITED_API))]
+    #[cfg(all(Py_3_10, Py_LIMITED_API, not(PyPy)))]
     #[cfg_attr(docsrs, doc(cfg(Py_3_10)))]
     pub fn Py_NewRef(obj: *mut PyObject) -> *mut PyObject;
-    #[cfg(all(Py_3_10, Py_LIMITED_API))]
+    #[cfg(all(Py_3_10, Py_LIMITED_API, not(PyPy)))]
     #[cfg_attr(docsrs, doc(cfg(Py_3_10)))]
     pub fn Py_XNewRef(obj: *mut PyObject) -> *mut PyObject;
 }
@@ -658,7 +658,7 @@ extern "C" {
 // macro _Py_NewRef not public; reimplemented directly inside Py_NewRef here
 // macro _Py_XNewRef not public; reimplemented directly inside Py_XNewRef here
 
-#[cfg(all(Py_3_10, not(Py_LIMITED_API)))]
+#[cfg(all(Py_3_10, any(not(Py_LIMITED_API), PyPy)))]
 #[cfg_attr(docsrs, doc(cfg(Py_3_10)))]
 #[inline]
 pub unsafe fn Py_NewRef(obj: *mut PyObject) -> *mut PyObject {
@@ -666,7 +666,7 @@ pub unsafe fn Py_NewRef(obj: *mut PyObject) -> *mut PyObject {
     obj
 }
 
-#[cfg(all(Py_3_10, not(Py_LIMITED_API)))]
+#[cfg(all(Py_3_10, any(not(Py_LIMITED_API), PyPy)))]
 #[cfg_attr(docsrs, doc(cfg(Py_3_10)))]
 #[inline]
 pub unsafe fn Py_XNewRef(obj: *mut PyObject) -> *mut PyObject {