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

Support conversion between num-bigint <-> Python Long #608

Merged
merged 6 commits into from
Oct 7, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ libc = "0.2.62"
spin = "0.5.1"
num-traits = "0.2.8"
pyo3cls = { path = "pyo3cls", version = "=0.8.0" }
num-complex = { version = "0.2.3", optional = true }
num-complex = { version = ">= 0.2", optional = true }
num-bigint = { version = ">= 0.2", optional = true }
inventory = "0.1.4"
indoc = "0.3.4"
unindent = "0.1.4"
Expand Down
2 changes: 1 addition & 1 deletion ci/appveyor/test.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ function Invoke-Call
}
}

Invoke-Call { cargo test --verbose }
Invoke-Call { cargo test --verbose --features="num-bigint num-complex" }

$examplesDirectory = "examples"

Expand Down
2 changes: 1 addition & 1 deletion ci/travis/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ set -ex

# run `cargo test` only if testing against cpython.
if ! [[ $FEATURES == *"pypy"* ]]; then
cargo test --features "$FEATURES num-complex"
cargo test --features "$FEATURES num-bigint num-complex"
( cd pyo3-derive-backend; cargo test )
else
# check that pypy at least builds
Expand Down
2 changes: 2 additions & 0 deletions src/ffi/longobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ extern "C" {
#[cfg(not(Py_LIMITED_API))]
#[cfg_attr(windows, link(name = "pythonXY"))]
extern "C" {
pub fn _PyLong_NumBits(obj: *mut PyObject) -> c_int;

#[cfg_attr(PyPy, link_name = "_PyPyLong_FromByteArray")]
pub fn _PyLong_FromByteArray(
bytes: *const c_uchar,
Expand Down
251 changes: 203 additions & 48 deletions src/types/num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::i64;
use std::os::raw::c_int;
use std::os::raw::{c_long, c_uchar};

pub(super) fn err_if_invalid_value<T: PartialEq + Copy>(
fn err_if_invalid_value<T: PartialEq>(
py: Python,
invalid_value: T,
actual_value: T,
Expand All @@ -29,9 +29,8 @@ pub(super) fn err_if_invalid_value<T: PartialEq + Copy>(
}
}

#[macro_export]
macro_rules! int_fits_larger_int (
($rust_type:ty, $larger_type:ty) => (
macro_rules! int_fits_larger_int {
($rust_type:ty, $larger_type:ty) => {
impl ToPyObject for $rust_type {
#[inline]
fn to_object(&self, py: Python) -> PyObject {
Expand All @@ -49,17 +48,22 @@ macro_rules! int_fits_larger_int (
let val = $crate::objectprotocol::ObjectProtocol::extract::<$larger_type>(obj)?;
match cast::<$larger_type, $rust_type>(val) {
Some(v) => Ok(v),
None => Err(exceptions::OverflowError.into())
None => Err(exceptions::OverflowError.into()),
}
}
}
)
);
};
}

// manual implementation for 128bit integers
#[cfg(target_endian = "little")]
const IS_LITTLE_ENDIAN: c_int = 1;
#[cfg(not(target_endian = "little"))]
const IS_LITTLE_ENDIAN: c_int = 0;

// for 128bit Integers
#[macro_export]
macro_rules! int_convert_bignum (
($rust_type: ty, $byte_size: expr, $is_little_endian: expr, $is_signed: expr) => (
macro_rules! int_convert_128 {
($rust_type: ty, $byte_size: expr, $is_signed: expr) => {
impl ToPyObject for $rust_type {
#[inline]
fn to_object(&self, py: Python) -> PyObject {
Expand All @@ -69,14 +73,11 @@ macro_rules! int_convert_bignum (
impl IntoPy<PyObject> for $rust_type {
fn into_py(self, py: Python) -> PyObject {
unsafe {
// TODO: Replace this with functions from the from_bytes family
// Once they are stabilized
// https://github.com/rust-lang/rust/issues/52963
let bytes = ::std::mem::transmute::<_, [c_uchar; $byte_size]>(self);
let bytes = self.to_ne_bytes();
let obj = ffi::_PyLong_FromByteArray(
bytes.as_ptr() as *const c_uchar,
$byte_size,
$is_little_endian,
IS_LITTLE_ENDIAN,
$is_signed,
);
PyObject::from_owned_ptr_or_panic(py, obj)
Expand All @@ -88,32 +89,26 @@ macro_rules! int_convert_bignum (
unsafe {
let num = ffi::PyNumber_Index(ob.as_ptr());
if num.is_null() {
return Err(PyErr::fetch(ob.py()));
return Err(PyErr::fetch(ob.py()));
}
let buffer: [c_uchar; $byte_size] = [0; $byte_size];
let ok = ffi::_PyLong_AsByteArray(
ob.as_ptr() as *mut ffi::PyLongObject,
buffer.as_ptr() as *const c_uchar,
$byte_size,
$is_little_endian,
IS_LITTLE_ENDIAN,
$is_signed,
);
if ok == -1 {
Err(PyErr::fetch(ob.py()))
} else {
Ok(::std::mem::transmute::<_, $rust_type>(buffer))
Ok(<$rust_type>::from_ne_bytes(buffer))
}
}
}
}
)
);

// manual implementation for 128bit integers
#[cfg(target_endian = "little")]
pub(super) const IS_LITTLE_ENDIAN: c_int = 1;
#[cfg(not(target_endian = "little"))]
pub(super) const IS_LITTLE_ENDIAN: c_int = 0;
};
}

/// Represents a Python `int` object.
///
Expand All @@ -131,18 +126,18 @@ pyobject_native_type!(
ffi::PyLong_Check
);

macro_rules! int_fits_c_long (
($rust_type:ty) => (
macro_rules! int_fits_c_long {
($rust_type:ty) => {
impl ToPyObject for $rust_type {
#![cfg_attr(feature="cargo-clippy", allow(clippy::cast_lossless))]
#![cfg_attr(feature = "cargo-clippy", allow(clippy::cast_lossless))]
fn to_object(&self, py: Python) -> PyObject {
unsafe {
PyObject::from_owned_ptr_or_panic(py, ffi::PyLong_FromLong(*self as c_long))
}
}
}
impl IntoPy<PyObject> for $rust_type {
#![cfg_attr(feature="cargo-clippy", allow(clippy::cast_lossless))]
#![cfg_attr(feature = "cargo-clippy", allow(clippy::cast_lossless))]
fn into_py(self, py: Python) -> PyObject {
unsafe {
PyObject::from_owned_ptr_or_panic(py, ffi::PyLong_FromLong(self as c_long))
Expand All @@ -165,34 +160,29 @@ macro_rules! int_fits_c_long (
}?;
match cast::<c_long, $rust_type>(val) {
Some(v) => Ok(v),
None => Err(exceptions::OverflowError.into())
None => Err(exceptions::OverflowError.into()),
}
}
}
)
);
};
}

macro_rules! int_convert_u64_or_i64 (
($rust_type:ty, $pylong_from_ll_or_ull:expr, $pylong_as_ll_or_ull:expr) => (
macro_rules! int_convert_u64_or_i64 {
($rust_type:ty, $pylong_from_ll_or_ull:expr, $pylong_as_ll_or_ull:expr) => {
impl ToPyObject for $rust_type {
#[inline]
fn to_object(&self, py: Python) -> PyObject {
unsafe {
PyObject::from_owned_ptr_or_panic(py, $pylong_from_ll_or_ull(*self))
}
unsafe { PyObject::from_owned_ptr_or_panic(py, $pylong_from_ll_or_ull(*self)) }
}
}
impl IntoPy<PyObject> for $rust_type {
#[inline]
fn into_py(self, py: Python) -> PyObject {
unsafe {
PyObject::from_owned_ptr_or_panic(py, $pylong_from_ll_or_ull(self))
}
unsafe { PyObject::from_owned_ptr_or_panic(py, $pylong_from_ll_or_ull(self)) }
}
}
impl<'source> FromPyObject<'source> for $rust_type {
fn extract(ob: &'source PyAny) -> PyResult<$rust_type>
{
fn extract(ob: &'source PyAny) -> PyResult<$rust_type> {
let ptr = ob.as_ptr();
unsafe {
let num = ffi::PyNumber_Index(ptr);
Expand All @@ -206,8 +196,8 @@ macro_rules! int_convert_u64_or_i64 (
}
}
}
)
);
};
}

int_fits_c_long!(i8);
int_fits_c_long!(u8);
Expand Down Expand Up @@ -242,10 +232,175 @@ int_convert_u64_or_i64!(
ffi::PyLong_AsUnsignedLongLong
);

#[cfg(all(not(Py_LIMITED_API), target_endian = "little"))]
int_convert_128!(i128, 16, 1);
#[cfg(not(Py_LIMITED_API))]
int_convert_bignum!(i128, 16, IS_LITTLE_ENDIAN, 1);
#[cfg(not(Py_LIMITED_API))]
int_convert_bignum!(u128, 16, IS_LITTLE_ENDIAN, 0);
int_convert_128!(u128, 16, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

i128 conversions on big-endian appears to have been inadvertently deleted

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it's at line 236

Copy link
Contributor

Choose a reason for hiding this comment

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

line 236 is only enabled on little-endian systems, so it can't provide the i128 conversions on big-endian systems since it is disabled.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I understand.
Thank you for pointing it out!


#[cfg(all(feature = "num-bigint", not(Py_LIMITED_API)))]
mod bitint_conversion {
Copy link
Member

Choose a reason for hiding this comment

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

typo?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks.

use super::*;
use num_bigint::{BigInt, BigUint};

unsafe fn extract_small(ob: &PyAny, n: usize, is_signed: c_int) -> PyResult<[c_uchar; 128]> {
let buffer = [0; 128];
let ok = ffi::_PyLong_AsByteArray(
ob.as_ptr() as *mut ffi::PyLongObject,
buffer.as_ptr() as *const c_uchar,
n,
1,
is_signed,
);
if ok == -1 {
Err(PyErr::fetch(ob.py()))
} else {
Ok(buffer)
}
}

unsafe fn extract_large(ob: &PyAny, n: usize, is_signed: c_int) -> PyResult<Vec<c_uchar>> {
let buffer = vec![0; n];
let ok = ffi::_PyLong_AsByteArray(
ob.as_ptr() as *mut ffi::PyLongObject,
buffer.as_ptr() as *const c_uchar,
n,
1,
is_signed,
);
if ok == -1 {
Err(PyErr::fetch(ob.py()))
} else {
Ok(buffer)
}
}

macro_rules! bigint_conversion {
($rust_ty: ty, $is_signed: expr, $to_bytes: path, $from_bytes: path) => {
impl ToPyObject for $rust_ty {
fn to_object(&self, py: Python) -> PyObject {
unsafe {
let bytes = $to_bytes(self);
let obj = ffi::_PyLong_FromByteArray(
bytes.as_ptr() as *const c_uchar,
bytes.len(),
1,
$is_signed,
);
PyObject::from_owned_ptr_or_panic(py, obj)
}
}
}
impl<'source> FromPyObject<'source> for $rust_ty {
fn extract(ob: &'source PyAny) -> PyResult<$rust_ty> {
unsafe {
let num = ffi::PyNumber_Index(ob.as_ptr());
if num.is_null() {
return Err(PyErr::fetch(ob.py()));
}
let n_bytes = (ffi::_PyLong_NumBits(ob.as_ptr()) as usize - 1) / 8 + 1;
Copy link
Member

Choose a reason for hiding this comment

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

looks like this should use num, not ob...

Can _PyLong_NumBits return zero? The subtract would wrap in that case.

Copy link
Member Author

Choose a reason for hiding this comment

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

looks like this should use num, not ob...

Wow, I didn't know that.

Can _PyLong_NumBits return zero? The subtract would wrap in that case.

Good catch, thanks.

if n_bytes <= 128 {
extract_small(ob, n_bytes, $is_signed)
.map(|b| $from_bytes(&b[..n_bytes]))
} else {
extract_large(ob, n_bytes, $is_signed).map(|b| $from_bytes(&b))
}
}
}
}
};
}
bigint_conversion!(BigUint, 0, BigUint::to_bytes_le, BigUint::from_bytes_le);
bigint_conversion!(
BigInt,
1,
BigInt::to_signed_bytes_le,
BigInt::from_signed_bytes_le
);

#[cfg(test)]
mod test {
use super::*;
use crate::types::{PyDict, PyModule};
use indoc::indoc;
use num_traits::{One, Zero};

fn python_fib(py: Python) -> &PyModule {
let fib_code = indoc!(
r#"
def fib(n):
f0, f1 = 0, 1
for _ in range(n):
f0, f1 = f1, f0 + f1
return f0

def fib_neg(n):
return -fib(n)
"#
);
PyModule::from_code(py, fib_code, "fib.py", "fib").unwrap()
}

fn rust_fib<T>(n: usize) -> T
where
T: Zero + One,
for<'a> &'a T: std::ops::Add<Output = T>,
{
let mut f0: T = Zero::zero();
let mut f1: T = One::one();
for _ in 0..n {
let f2 = &f0 + &f1;
f0 = std::mem::replace(&mut f1, f2);
}
f0
}

#[test]
fn convert_biguint() {
let gil = Python::acquire_gil();
let py = gil.python();
let rs_result: BigUint = rust_fib(400);
let fib = python_fib(py);
let locals = PyDict::new(py);
locals.set_item("rs_result", &rs_result).unwrap();
locals.set_item("fib", fib).unwrap();
// Checks if Rust BigUint -> Python Long conversion is correct
py.run("assert fib.fib(400) == rs_result", None, Some(locals))
.unwrap();
// Checks if Python Long -> Rust BigUint conversion is correct if N is small
let py_result: BigUint =
FromPyObject::extract(fib.call1("fib", (400,)).unwrap()).unwrap();
assert_eq!(rs_result, py_result);
// Checks if Python Long -> Rust BigUint conversion is correct if N is large
let rs_result: BigUint = rust_fib(2000);
let py_result: BigUint =
FromPyObject::extract(fib.call1("fib", (2000,)).unwrap()).unwrap();
assert_eq!(rs_result, py_result);
}

#[test]
fn convert_bigint() {
let gil = Python::acquire_gil();
let py = gil.python();
let rs_result = rust_fib::<BigInt>(400) * -1;
let fib = python_fib(py);
let locals = PyDict::new(py);
locals.set_item("rs_result", &rs_result).unwrap();
locals.set_item("fib", fib).unwrap();
// Checks if Rust BigInt -> Python Long conversion is correct
py.run("assert fib.fib_neg(400) == rs_result", None, Some(locals))
.unwrap();
// Checks if Python Long -> Rust BigInt conversion is correct if N is small
let py_result: BigInt =
FromPyObject::extract(fib.call1("fib_neg", (400,)).unwrap()).unwrap();
assert_eq!(rs_result, py_result);
// Checks if Python Long -> Rust BigInt conversion is correct if N is large
let rs_result = rust_fib::<BigInt>(2000) * -1;
let py_result: BigInt =
FromPyObject::extract(fib.call1("fib_neg", (2000,)).unwrap()).unwrap();
assert_eq!(rs_result, py_result);
}
}
}

#[cfg(test)]
mod test {
Expand Down