From 03ba4a55975a528897a89f836dcc16bb12237173 Mon Sep 17 00:00:00 2001
From: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
Date: Sun, 26 Sep 2021 13:45:47 +0100
Subject: [PATCH] ffi: use _Py_NewRef for clarity

---
 src/class/impl_.rs            | 12 ++++--------
 src/conversion.rs             |  8 +-------
 src/types/dict.rs             | 10 +++++-----
 src/types/module.rs           |  3 +--
 src/types/set.rs              |  3 +--
 tests/test_buffer.rs          |  3 +--
 tests/test_buffer_protocol.rs |  3 +--
 7 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/src/class/impl_.rs b/src/class/impl_.rs
index dc38c038..de3e5812 100644
--- a/src/class/impl_.rs
+++ b/src/class/impl_.rs
@@ -257,8 +257,7 @@ macro_rules! define_pyclass_binary_operator_slot {
                 _slf: *mut ffi::PyObject,
                 _other: *mut ffi::PyObject,
             ) -> PyResult<*mut ffi::PyObject> {
-                ffi::Py_INCREF(ffi::Py_NotImplemented());
-                Ok(ffi::Py_NotImplemented())
+                Ok(ffi::_Py_NewRef(ffi::Py_NotImplemented()))
             }
         }
 
@@ -273,8 +272,7 @@ macro_rules! define_pyclass_binary_operator_slot {
                 _slf: *mut ffi::PyObject,
                 _other: *mut ffi::PyObject,
             ) -> PyResult<*mut ffi::PyObject> {
-                ffi::Py_INCREF(ffi::Py_NotImplemented());
-                Ok(ffi::Py_NotImplemented())
+                Ok(ffi::_Py_NewRef(ffi::Py_NotImplemented()))
             }
         }
 
@@ -429,8 +427,7 @@ slot_fragment_trait! {
         _other: *mut ffi::PyObject,
         _mod: *mut ffi::PyObject,
     ) -> PyResult<*mut ffi::PyObject> {
-        ffi::Py_INCREF(ffi::Py_NotImplemented());
-        Ok(ffi::Py_NotImplemented())
+        Ok(ffi::_Py_NewRef(ffi::Py_NotImplemented()))
     }
 }
 
@@ -446,8 +443,7 @@ slot_fragment_trait! {
         _other: *mut ffi::PyObject,
         _mod: *mut ffi::PyObject,
     ) -> PyResult<*mut ffi::PyObject> {
-        ffi::Py_INCREF(ffi::Py_NotImplemented());
-        Ok(ffi::Py_NotImplemented())
+        Ok(ffi::_Py_NewRef(ffi::Py_NotImplemented()))
     }
 }
 
diff --git a/src/conversion.rs b/src/conversion.rs
index 0463d670..a0fb7fd4 100644
--- a/src/conversion.rs
+++ b/src/conversion.rs
@@ -64,13 +64,7 @@ where
     T: AsPyPointer,
 {
     fn into_ptr(self) -> *mut ffi::PyObject {
-        let ptr = self.as_ptr();
-        if !ptr.is_null() {
-            unsafe {
-                ffi::Py_INCREF(ptr);
-            }
-        }
-        ptr
+        unsafe { ffi::_Py_XNewRef(self.as_ptr()) }
     }
 }
 
diff --git a/src/types/dict.rs b/src/types/dict.rs
index 89e29bb4..698d727a 100644
--- a/src/types/dict.rs
+++ b/src/types/dict.rs
@@ -105,8 +105,7 @@ impl PyDict {
             let ptr = ffi::PyDict_GetItem(self.as_ptr(), key);
             NonNull::new(ptr).map(|p| {
                 // PyDict_GetItem return s borrowed ptr, must make it owned for safety (see #890).
-                ffi::Py_INCREF(p.as_ptr());
-                self.py().from_owned_ptr(p.as_ptr())
+                self.py().from_owned_ptr(ffi::_Py_NewRef(p.as_ptr()))
             })
         })
     }
@@ -196,9 +195,10 @@ impl<'py> Iterator for PyDictIterator<'py> {
             if ffi::PyDict_Next(self.dict.as_ptr(), &mut self.pos, &mut key, &mut value) != 0 {
                 let py = self.dict.py();
                 // PyDict_Next returns borrowed values; for safety must make them owned (see #890)
-                ffi::Py_INCREF(key);
-                ffi::Py_INCREF(value);
-                Some((py.from_owned_ptr(key), py.from_owned_ptr(value)))
+                Some((
+                    py.from_owned_ptr(ffi::_Py_NewRef(key)),
+                    py.from_owned_ptr(ffi::_Py_NewRef(value)),
+                ))
             } else {
                 None
             }
diff --git a/src/types/module.rs b/src/types/module.rs
index 97bdef36..e3cd30f2 100644
--- a/src/types/module.rs
+++ b/src/types/module.rs
@@ -140,8 +140,7 @@ impl PyModule {
         unsafe {
             // PyModule_GetDict returns borrowed ptr; must make owned for safety (see #890).
             let ptr = ffi::PyModule_GetDict(self.as_ptr());
-            ffi::Py_INCREF(ptr);
-            self.py().from_owned_ptr(ptr)
+            self.py().from_owned_ptr(ffi::_Py_NewRef(ptr))
         }
     }
 
diff --git a/src/types/set.rs b/src/types/set.rs
index 01ffafba..8650e99c 100644
--- a/src/types/set.rs
+++ b/src/types/set.rs
@@ -164,8 +164,7 @@ impl<'py> Iterator for PySetIterator<'py> {
             let mut hash: ffi::Py_hash_t = 0;
             if ffi::_PySet_NextEntry(self.set.as_ptr(), &mut self.pos, &mut key, &mut hash) != 0 {
                 // _PySet_NextEntry returns borrowed object; for safety must make owned (see #890)
-                ffi::Py_INCREF(key);
-                Some(self.set.py().from_owned_ptr(key))
+                Some(self.set.py().from_owned_ptr(ffi::_Py_NewRef(key)))
             } else {
                 None
             }
diff --git a/tests/test_buffer.rs b/tests/test_buffer.rs
index 462cc556..bcdfda1c 100644
--- a/tests/test_buffer.rs
+++ b/tests/test_buffer.rs
@@ -39,8 +39,7 @@ impl PyBufferProtocol for TestBufferErrors {
         }
 
         unsafe {
-            (*view).obj = slf.as_ptr();
-            ffi::Py_INCREF((*view).obj);
+            (*view).obj = ffi::_Py_NewRef(slf.as_ptr());
         }
 
         let bytes = &slf.buf;
diff --git a/tests/test_buffer_protocol.rs b/tests/test_buffer_protocol.rs
index e5831d1b..f7174449 100644
--- a/tests/test_buffer_protocol.rs
+++ b/tests/test_buffer_protocol.rs
@@ -33,8 +33,7 @@ impl PyBufferProtocol for TestBufferClass {
         }
 
         unsafe {
-            (*view).obj = slf.as_ptr();
-            ffi::Py_INCREF((*view).obj);
+            (*view).obj = ffi::_Py_NewRef(slf.as_ptr());
         }
 
         let bytes = &slf.vec;