Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impl partialeq for pylong #4317

Merged
merged 10 commits into from
Jul 27, 2024
1 change: 1 addition & 0 deletions newsfragments/4317.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Implement `PartialEq` for `Bound<'py, PyLong>` with `u8`, `u16`, `u32`, `u64`, `u128`, `usize`, `i8`, `i16`, `i32`, `i64`, `i128` and `isize`.
4 changes: 3 additions & 1 deletion src/sealed.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::types::{
PyBool, PyByteArray, PyBytes, PyCapsule, PyComplex, PyDict, PyFloat, PyFrozenSet, PyList,
PyMapping, PyModule, PySequence, PySet, PySlice, PyString, PyTraceback, PyTuple, PyType,
PyLong, PyMapping, PyModule, PySequence, PySet, PySlice, PyString, PyTraceback, PyTuple,
PyType,
};
use crate::{ffi, Bound, PyAny, PyResult};

Expand All @@ -23,6 +24,7 @@ impl Sealed for Bound<'_, PyDict> {}
impl Sealed for Bound<'_, PyFloat> {}
impl Sealed for Bound<'_, PyFrozenSet> {}
impl Sealed for Bound<'_, PyList> {}
impl Sealed for Bound<'_, PyLong> {}
impl Sealed for Bound<'_, PyMapping> {}
impl Sealed for Bound<'_, PyModule> {}
impl Sealed for Bound<'_, PySequence> {}
Expand Down
301 changes: 300 additions & 1 deletion src/types/num.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use crate::{ffi, PyAny};
use super::any::PyAnyMethods;

use crate::{ffi, ffi_ptr_ext::FfiPtrExt, instance::Bound, PyAny, Python};
use std::os::raw::c_long;

/// Represents a Python `int` object.
///
Expand All @@ -10,3 +13,299 @@ use crate::{ffi, PyAny};
pub struct PyLong(PyAny);

pyobject_native_type_core!(PyLong, pyobject_native_static_type_object!(ffi::PyLong_Type), #checkfunction=ffi::PyLong_Check);

impl PyLong {
/// Creates a new `PyLong` from a C `long` integer.
/// # Returns
/// A [`Bound`] reference to a `PyLong` object representing the given `val`.
#[inline]
pub fn new_bound_from_c_long(py: Python<'_>, val: c_long) -> Bound<'_, Self> {
unsafe {
ffi::PyLong_FromLong(val)
.assume_owned(py)
.downcast_into_unchecked()
}
}
}

/// Implementation of functionality for [`PyLong`].
///
/// These methods are defined for the `Bound<'py, PyLong>` smart pointer, so to use method call
/// syntax these methods are separated into a trait, because stable Rust does not yet support
/// `arbitrary_self_types`.
#[doc(alias = "PyLong")]
pub trait PyLongMethods<'py>: crate::sealed::Sealed {
/// Gets the value of this int.
fn value(&self) -> c_long;
}

impl<'py> PyLongMethods<'py> for Bound<'py, PyLong> {
fn value(&self) -> c_long {
unsafe {
// Safety: self is PyLong object
ffi::PyLong_AsLong(self.as_ptr())
}
}
}

/// Implement <Bound<'_, PyLong>> == i8 comparisons
impl PartialEq<i8> for Bound<'_, PyLong> {
#[inline]
fn eq(&self, other: &i8) -> bool {
self.value() == *other as c_long
}
}

/// Implement i8 == <Bound<'_, PyLong>> comparisons
impl PartialEq<Bound<'_, PyLong>> for i8 {
#[inline]
fn eq(&self, other: &Bound<'_, PyLong>) -> bool {
*self as c_long == other.value()
}
}

/// Implement <Bound<'_, PyLong>> == u8 comparisons
impl PartialEq<u8> for Bound<'_, PyLong> {
#[inline]
fn eq(&self, other: &u8) -> bool {
self.value() == *other as c_long
}
}

/// Implement u8 == <Bound<'_, PyLong>> comparisons
impl PartialEq<Bound<'_, PyLong>> for u8 {
#[inline]
fn eq(&self, other: &Bound<'_, PyLong>) -> bool {
*self as c_long == other.value()
}
}

/// Implement <Bound<'_, PyLong>> == i16 comparisons
impl PartialEq<i16> for Bound<'_, PyLong> {
#[inline]
fn eq(&self, other: &i16) -> bool {
self.value() == *other as c_long
}
}

/// Implement i16 == <Bound<'_, PyLong>> comparisons
impl PartialEq<Bound<'_, PyLong>> for i16 {
#[inline]
fn eq(&self, other: &Bound<'_, PyLong>) -> bool {
*self as c_long == other.value()
}
}

/// Implement <Bound<'_, PyLong>> == u16 comparisons
impl PartialEq<u16> for Bound<'_, PyLong> {
#[inline]
fn eq(&self, other: &u16) -> bool {
self.value() == *other as c_long
}
}

/// Implement u16 == <Bound<'_, PyLong>> comparisons
impl PartialEq<Bound<'_, PyLong>> for u16 {
#[inline]
fn eq(&self, other: &Bound<'_, PyLong>) -> bool {
*self as c_long == other.value()
}
}

/// Implement <Bound<'_, PyLong>> == i32 comparisons
impl PartialEq<i32> for Bound<'_, PyLong> {
#[inline]
fn eq(&self, other: &i32) -> bool {
self.value() == *other as c_long
}
}

/// Implement i32 == <Bound<'_, PyLong>> comparisons
impl PartialEq<Bound<'_, PyLong>> for i32 {
#[inline]
fn eq(&self, other: &Bound<'_, PyLong>) -> bool {
*self as c_long == other.value()
}
}

/// Implement <Bound<'_, PyLong>> == u32 comparisons
impl PartialEq<u32> for Bound<'_, PyLong> {
#[inline]
fn eq(&self, other: &u32) -> bool {
self.value() == *other as c_long
}
}

/// Implement u32 == <Bound<'_, PyLong>> comparisons
impl PartialEq<Bound<'_, PyLong>> for u32 {
#[inline]
fn eq(&self, other: &Bound<'_, PyLong>) -> bool {
*self as c_long == other.value()
}
}

/// Implement <Bound<'_, PyLong>> == i64 comparisons
impl PartialEq<i64> for Bound<'_, PyLong> {
#[inline]
fn eq(&self, other: &i64) -> bool {
self.value() == *other as c_long
}
}

/// Implement i64 == <Bound<'_, PyLong>> comparisons
impl PartialEq<Bound<'_, PyLong>> for i64 {
#[inline]
fn eq(&self, other: &Bound<'_, PyLong>) -> bool {
*self as c_long == other.value()
}
}

/// Implement <Bound<'_, PyLong>> == u64 comparisons
impl PartialEq<u64> for Bound<'_, PyLong> {
#[inline]
fn eq(&self, other: &u64) -> bool {
self.value() == *other as c_long
}
}

/// Implement u64 == <Bound<'_, PyLong>> comparisons
impl PartialEq<Bound<'_, PyLong>> for u64 {
#[inline]
fn eq(&self, other: &Bound<'_, PyLong>) -> bool {
*self as c_long == other.value()
}
}

/// Implement <Bound<'_, PyLong>> == i128 comparisons
impl PartialEq<i128> for Bound<'_, PyLong> {
#[inline]
fn eq(&self, other: &i128) -> bool {
self.value() == *other as c_long
}
}

/// Implement i128 == <Bound<'_, PyLong>> comparisons
impl PartialEq<Bound<'_, PyLong>> for i128 {
#[inline]
fn eq(&self, other: &Bound<'_, PyLong>) -> bool {
*self as c_long == other.value()
}
}

/// Implement <Bound<'_, PyLong>> == u128 comparisons
impl PartialEq<u128> for Bound<'_, PyLong> {
#[inline]
fn eq(&self, other: &u128) -> bool {
self.value() == *other as c_long
}
}

/// Implement u128 == <Bound<'_, PyLong>> comparisons
impl PartialEq<Bound<'_, PyLong>> for u128 {
#[inline]
fn eq(&self, other: &Bound<'_, PyLong>) -> bool {
*self as c_long == other.value()
}
}

/// Implement <Bound<'_, PyLong>> == isize comparisons
impl PartialEq<isize> for Bound<'_, PyLong> {
#[inline]
fn eq(&self, other: &isize) -> bool {
self.value() == *other as c_long
}
}

/// Implement isize == <Bound<'_, PyLong>> comparisons
impl PartialEq<Bound<'_, PyLong>> for isize {
#[inline]
fn eq(&self, other: &Bound<'_, PyLong>) -> bool {
*self as c_long == other.value()
}
}

/// Implement <Bound<'_, PyLong>> == usize comparisons
impl PartialEq<usize> for Bound<'_, PyLong> {
#[inline]
fn eq(&self, other: &usize) -> bool {
self.value() == *other as c_long
}
}

/// Implement usize == <Bound<'_, PyLong>> comparisons
impl PartialEq<Bound<'_, PyLong>> for usize {
#[inline]
fn eq(&self, other: &Bound<'_, PyLong>) -> bool {
*self as c_long == other.value()
}
}

#[cfg(test)]
mod tests {
use super::PyLong;
use super::PyLongMethods;
use crate::Python;

#[test]
fn test_c_long_value() {
Python::with_gil(|py| {
let v = 123;
let obj = PyLong::new_bound_from_c_long(py, 123);
assert_eq!(v, obj.value());
});
}

#[test]
fn test_partial_eq() {
Python::with_gil(|py| {
let v_i8 = 123i8;
let v_u8 = 123i8;
let v_i16 = 123i16;
let v_u16 = 123u16;
let v_i32 = 123i32;
let v_u32 = 123u32;
let v_i64 = 123i64;
let v_u64 = 123u64;
let v_i128 = 123i128;
let v_u128 = 123u128;
let v_isize = 123isize;
let v_usize = 123usize;
let obj = PyLong::new_bound_from_c_long(py, 123);
assert_eq!(v_i8, obj);
assert_eq!(obj, v_i8);

assert_eq!(v_u8, obj);
assert_eq!(obj, v_u8);

assert_eq!(v_i16, obj);
assert_eq!(obj, v_i16);

assert_eq!(v_u16, obj);
assert_eq!(obj, v_u16);

assert_eq!(v_i32, obj);
assert_eq!(obj, v_i32);

assert_eq!(v_u32, obj);
assert_eq!(obj, v_u32);

assert_eq!(v_i64, obj);
assert_eq!(obj, v_i64);

assert_eq!(v_u64, obj);
assert_eq!(obj, v_u64);

assert_eq!(v_i128, obj);
assert_eq!(obj, v_i128);

assert_eq!(v_u128, obj);
assert_eq!(obj, v_u128);

assert_eq!(v_isize, obj);
assert_eq!(obj, v_isize);

assert_eq!(v_usize, obj);
assert_eq!(obj, v_usize);
});
}
}
2 changes: 1 addition & 1 deletion tests/test_class_attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ macro_rules! test_case {
let struct_obj = struct_class.call0().unwrap();
assert!(struct_obj.setattr($renamed_field_name, 2).is_ok());
let attr = struct_obj.getattr($renamed_field_name).unwrap();
assert_eq!(2, attr.extract().unwrap());
assert_eq!(2, attr.extract::<u8>().unwrap());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So just completing my read of the full diff, this is interesting.

What I see here is that the introduction of these implementations has changed type inference here. It's a potentially breaking change.

Is it a problem? I don't think so; user code could already define other types implementing FromPyObject and PartialEq<u8> and thus have the same inference failure.

And I believe that this new PartialEq implementation here still holds potential convenience and usability improvements.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see how this can be a breaking change. However, I don't think it's a major issue.

Ultimately, I'll leave the final decision to the maintainers haha 😃 .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I think we move ahead with this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't consider inference failures like this as breaking changes, so I'm also fine with this.

});
}
};
Expand Down
Loading