From 43477a8e306aa408c6c796095fda45ed17f90cd5 Mon Sep 17 00:00:00 2001
From: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
Date: Tue, 4 Jul 2023 20:34:23 +0100
Subject: [PATCH] use error_on_minusone in more cases

---
 src/conversions/std/osstr.rs |  6 +-----
 src/err/mod.rs               | 24 +++++++++++++++++++++---
 src/types/any.rs             | 14 ++++----------
 src/types/mapping.rs         |  7 ++-----
 src/types/sequence.rs        | 21 ++++++---------------
 5 files changed, 34 insertions(+), 38 deletions(-)

diff --git a/src/conversions/std/osstr.rs b/src/conversions/std/osstr.rs
index 12d00ffc..f91822a8 100644
--- a/src/conversions/std/osstr.rs
+++ b/src/conversions/std/osstr.rs
@@ -1,6 +1,4 @@
 use crate::types::PyString;
-#[cfg(windows)]
-use crate::PyErr;
 use crate::{
     ffi, AsPyPointer, FromPyObject, IntoPy, PyAny, PyObject, PyResult, Python, ToPyObject,
 };
@@ -92,9 +90,7 @@ impl FromPyObject<'_> for OsString {
             // ourselves
             let size =
                 unsafe { ffi::PyUnicode_AsWideChar(pystring.as_ptr(), std::ptr::null_mut(), 0) };
-            if size == -1 {
-                return Err(PyErr::fetch(ob.py()));
-            }
+            crate::err::error_on_minusone(ob.py(), size)?;
 
             let mut buffer = vec![0; size as usize];
             let bytes_read =
diff --git a/src/err/mod.rs b/src/err/mod.rs
index 05072601..9eee0a67 100644
--- a/src/err/mod.rs
+++ b/src/err/mod.rs
@@ -9,7 +9,6 @@ use crate::{AsPyPointer, IntoPy, IntoPyPointer, Py, PyAny, PyObject, Python, ToP
 use std::borrow::Cow;
 use std::cell::UnsafeCell;
 use std::ffi::CString;
-use std::os::raw::c_int;
 
 mod err_state;
 mod impls;
@@ -792,14 +791,33 @@ pub fn panic_after_error(_py: Python<'_>) -> ! {
 
 /// Returns Ok if the error code is not -1.
 #[inline]
-pub fn error_on_minusone(py: Python<'_>, result: c_int) -> PyResult<()> {
-    if result != -1 {
+pub(crate) fn error_on_minusone<T: SignedInteger>(py: Python<'_>, result: T) -> PyResult<()> {
+    if result != T::MINUS_ONE {
         Ok(())
     } else {
         Err(PyErr::fetch(py))
     }
 }
 
+pub(crate) trait SignedInteger: Eq {
+    const MINUS_ONE: Self;
+}
+
+macro_rules! impl_signed_integer {
+    ($t:ty) => {
+        impl SignedInteger for $t {
+            const MINUS_ONE: Self = -1;
+        }
+    };
+}
+
+impl_signed_integer!(i8);
+impl_signed_integer!(i16);
+impl_signed_integer!(i32);
+impl_signed_integer!(i64);
+impl_signed_integer!(i128);
+impl_signed_integer!(isize);
+
 #[inline]
 fn exceptions_must_derive_from_base_exception(py: Python<'_>) -> PyErr {
     PyErr::from_state(PyErrState::exceptions_must_derive_from_base_exception(py))
diff --git a/src/types/any.rs b/src/types/any.rs
index 5b08d496..69e2c3f5 100644
--- a/src/types/any.rs
+++ b/src/types/any.rs
@@ -947,11 +947,8 @@ impl PyAny {
     /// This is equivalent to the Python expression `hash(self)`.
     pub fn hash(&self) -> PyResult<isize> {
         let v = unsafe { ffi::PyObject_Hash(self.as_ptr()) };
-        if v == -1 {
-            Err(PyErr::fetch(self.py()))
-        } else {
-            Ok(v)
-        }
+        crate::err::error_on_minusone(self.py(), v)?;
+        Ok(v)
     }
 
     /// Returns the length of the sequence or mapping.
@@ -959,11 +956,8 @@ impl PyAny {
     /// This is equivalent to the Python expression `len(self)`.
     pub fn len(&self) -> PyResult<usize> {
         let v = unsafe { ffi::PyObject_Size(self.as_ptr()) };
-        if v == -1 {
-            Err(PyErr::fetch(self.py()))
-        } else {
-            Ok(v as usize)
-        }
+        crate::err::error_on_minusone(self.py(), v)?;
+        Ok(v as usize)
     }
 
     /// Returns the list of attributes of this object.
diff --git a/src/types/mapping.rs b/src/types/mapping.rs
index fa73176d..f580e7ff 100644
--- a/src/types/mapping.rs
+++ b/src/types/mapping.rs
@@ -17,11 +17,8 @@ impl PyMapping {
     #[inline]
     pub fn len(&self) -> PyResult<usize> {
         let v = unsafe { ffi::PyMapping_Size(self.as_ptr()) };
-        if v == -1 {
-            Err(PyErr::fetch(self.py()))
-        } else {
-            Ok(v as usize)
-        }
+        crate::err::error_on_minusone(self.py(), v)?;
+        Ok(v as usize)
     }
 
     /// Returns whether the mapping is empty.
diff --git a/src/types/sequence.rs b/src/types/sequence.rs
index b43bc60d..91ca7039 100644
--- a/src/types/sequence.rs
+++ b/src/types/sequence.rs
@@ -23,11 +23,8 @@ impl PySequence {
     #[inline]
     pub fn len(&self) -> PyResult<usize> {
         let v = unsafe { ffi::PySequence_Size(self.as_ptr()) };
-        if v == -1 {
-            Err(PyErr::fetch(self.py()))
-        } else {
-            Ok(v as usize)
-        }
+        crate::err::error_on_minusone(self.py(), v)?;
+        Ok(v as usize)
     }
 
     /// Returns whether the sequence is empty.
@@ -191,11 +188,8 @@ impl PySequence {
     {
         fn inner(seq: &PySequence, value: PyObject) -> PyResult<usize> {
             let r = unsafe { ffi::PySequence_Count(seq.as_ptr(), value.as_ptr()) };
-            if r == -1 {
-                Err(PyErr::fetch(seq.py()))
-            } else {
-                Ok(r as usize)
-            }
+            crate::err::error_on_minusone(seq.py(), r)?;
+            Ok(r as usize)
         }
 
         inner(self, value.to_object(self.py()))
@@ -231,11 +225,8 @@ impl PySequence {
     {
         fn inner(seq: &PySequence, value: PyObject) -> PyResult<usize> {
             let r = unsafe { ffi::PySequence_Index(seq.as_ptr(), value.as_ptr()) };
-            if r == -1 {
-                Err(PyErr::fetch(seq.py()))
-            } else {
-                Ok(r as usize)
-            }
+            crate::err::error_on_minusone(seq.py(), r)?;
+            Ok(r as usize)
         }
 
         inner(self, value.to_object(self.py()))