Skip to content
Open
Show file tree
Hide file tree
Changes from 10 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
244 changes: 237 additions & 7 deletions src/conversions/num_bigint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@
//! assert n + 1 == value
//! ```

#[cfg(all(not(Py_LIMITED_API), Py_3_14))]
use crate::conversions::std::num::{
is_30bit_layout, pylong_from_digits, pylong_visit_digits, PYLONG_BITS_IN_DIGIT,
};
#[cfg(Py_LIMITED_API)]
use crate::types::{bytes::PyBytesMethods, PyBytes};
use crate::{
Expand All @@ -67,7 +71,7 @@ use num_bigint::Sign;

// for identical functionality between BigInt and BigUint
macro_rules! bigint_conversion {
($rust_ty: ty, $is_signed: literal) => {
($rust_ty: ty, $is_signed: literal, $bits:path, $iter_u32_digits:path, $negative:expr) => {
#[cfg_attr(docsrs, doc(cfg(feature = "num-bigint")))]
impl<'py> IntoPyObject<'py> for $rust_ty {
type Target = PyInt;
Expand Down Expand Up @@ -95,6 +99,81 @@ macro_rules! bigint_conversion {
fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
use num_traits::ToBytes;

#[cfg(all(not(Py_LIMITED_API), Py_3_14))]
{
if is_30bit_layout() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some observations on this path:

  • I would prefer to have this logic extracted into a generic function; I think it's easier to maintain than when it's deeply nested inside a macro.
  • I think we can avoid the intermediate Vec allocation from the digits and instead wrap the iterator coming out of the big-integer. pylong_from_digits needs an ExactSizeIterator, so it'll be a touch fiddly but I think still possible.
  • I think we lack benchmarks for the bigint to-python pathway, so possibly worth landing some of those before we think too hard about this.

@chirizxc chirizxc Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  • I think we lack benchmarks for the bigint to-python pathway, so possibly worth landing some of those before we think too hard about this.

#6148

const MASK: u32 = (1 << PYLONG_BITS_IN_DIGIT) - 1;

let bits: usize = $bits(self)
.try_into()
.expect(concat!(stringify!($rust_ty), " bit length fits in usize"));

let py_digits_len = bits.div_ceil(PYLONG_BITS_IN_DIGIT).max(1);
let mut py_digits = Vec::with_capacity(py_digits_len);
let mut digits = $iter_u32_digits(self);
let ptr: *mut u32 = py_digits.as_mut_ptr();

if bits == 0 {
unsafe {
// SAFETY: `py_digits_len.max(1)` guarantees capacity for one elemetn
ptr.write(0);
py_digits.set_len(1);
}
} else {
// SAFETY: `bits != 0` means the integer has at least one digit
let last = unsafe { digits.next_back().unwrap_unchecked() };
let mut acc: u64 = 0;
let mut acc_bits: usize = 0;
let mut written: usize = 0;

for digit in digits {
acc |= u64::from(digit) << acc_bits;
let new_bits = acc_bits + u32::BITS as usize;

unsafe {
// SAFETY: the total number of writes is bounded by `py_digits_len`
ptr.add(written).write(acc as u32 & MASK);
written += 1;
}
acc >>= PYLONG_BITS_IN_DIGIT;

if new_bits >= PYLONG_BITS_IN_DIGIT * 2 {
unsafe {
// SAFETY: same bound as above for the optional second digit
ptr.add(written).write(acc as u32 & (MASK));
written += 1;
}
acc >>= PYLONG_BITS_IN_DIGIT;
acc_bits = new_bits - PYLONG_BITS_IN_DIGIT * 2;
} else {
acc_bits = new_bits - PYLONG_BITS_IN_DIGIT;
}
}

acc |= u64::from(last) << acc_bits;
while written < py_digits_len {
unsafe {
// SAFETY: `written < py_digits_len <= capacity` by construction
ptr.add(written).write(acc as u32 & (MASK));
written += 1;
}
acc >>= PYLONG_BITS_IN_DIGIT;
}

unsafe {
// SAFETY: exactly `written` elements were initialized above
py_digits.set_len(written);
}
}

return Ok(pylong_from_digits(
py,
$negative(self),
py_digits.into_iter(),
));
}
}

#[cfg(all(not(Py_LIMITED_API), Py_3_13))]
{
use crate::conversions::std::num::int_from_ne_bytes;
Expand Down Expand Up @@ -133,8 +212,20 @@ macro_rules! bigint_conversion {
};
}

bigint_conversion!(BigUint, false);
bigint_conversion!(BigInt, true);
bigint_conversion!(
BigUint,
false,
BigUint::bits,
BigUint::iter_u32_digits,
|_| false
);
bigint_conversion!(
BigInt,
true,
BigInt::bits,
BigInt::iter_u32_digits,
|value: &BigInt| BigInt::sign(value) == Sign::Minus
);

#[cfg_attr(docsrs, doc(cfg(feature = "num-bigint")))]
impl<'py> FromPyObject<'_, 'py> for BigInt {
Expand All @@ -154,6 +245,20 @@ impl<'py> FromPyObject<'_, 'py> for BigInt {
};
#[cfg(not(Py_LIMITED_API))]
{
#[cfg(Py_3_14)]
if is_30bit_layout() {
return pylong_visit_digits(
num.as_any().as_borrowed(),
|negative, compact, digits| {
let Some(digits) = digits else {
return Ok(BigInt::from(compact));
};
let sign = if negative { Sign::Minus } else { Sign::Plus };
Ok(BigInt::new(sign, int_from_pylong_digits(digits)))
},
);
}

let mut buffer = int_to_u32_vec::<true>(&num)?;
let sign = if buffer.last().copied().is_some_and(|last| last >> 31 != 0) {
// BigInt::new takes an unsigned array, so need to convert from two's complement
Expand Down Expand Up @@ -206,6 +311,24 @@ impl<'py> FromPyObject<'_, 'py> for BigUint {
};
#[cfg(not(Py_LIMITED_API))]
{
#[cfg(Py_3_14)]
if is_30bit_layout() {
return pylong_visit_digits(
num.as_any().as_borrowed(),
|negative, compact, digits| {
if negative {
return Err(crate::exceptions::PyValueError::new_err(
"can't convert negative int to unsigned",
));
}
let Some(digits) = digits else {
return Ok(BigUint::from(compact as u64));
};
Ok(BigUint::new(int_from_pylong_digits(digits)))
},
);
}

let buffer = int_to_u32_vec::<false>(&num)?;
Ok(BigUint::new(buffer))
}
Expand All @@ -221,6 +344,62 @@ impl<'py> FromPyObject<'_, 'py> for BigUint {
}
}

#[cfg(all(not(Py_LIMITED_API), Py_3_14))]
#[inline]
fn int_from_pylong_digits(digits: &[u32]) -> Vec<u32> {
let n_digits = digits
.last()
.map(|last| {
((digits.len() - 1) * PYLONG_BITS_IN_DIGIT
+ (u32::BITS as usize - last.leading_zeros() as usize))
Comment thread
chirizxc marked this conversation as resolved.
Outdated
.div_ceil(u32::BITS as usize)
})
.unwrap_or(0);

let mut py_digits = Vec::with_capacity(n_digits);
let ptr: *mut u32 = py_digits.as_mut_ptr();

if let Some((&last, init)) = digits.split_last() {
let mut acc = 0;
let mut acc_bits = 0;
let mut written = 0;

for &digit in init {
acc |= u64::from(digit) << acc_bits;
let new_bits = acc_bits + PYLONG_BITS_IN_DIGIT;

if new_bits >= u32::BITS as usize {
unsafe {
// SAFETY: the total number of writes is bounded by `n_digits`
ptr.add(written).write(acc as u32);
written += 1;
}
acc >>= u32::BITS;
acc_bits = new_bits - u32::BITS as usize;
} else {
acc_bits = new_bits;
}
}

acc |= u64::from(last) << acc_bits;
while written < n_digits {
unsafe {
// SAFETY: `written < n_digits <= capacity` by construction
ptr.add(written).write(acc as u32);
written += 1;
}
acc >>= u32::BITS;
}

unsafe {
// SAFETY: exactly `written` elements were initialized above
py_digits.set_len(written);
}
}

py_digits
}

#[cfg(not(any(Py_LIMITED_API, Py_3_13)))]
#[inline]
fn int_to_u32_vec<const SIGNED: bool>(long: &Bound<'_, PyInt>) -> PyResult<Vec<u32>> {
Expand Down Expand Up @@ -269,9 +448,7 @@ fn int_to_u32_vec<const SIGNED: bool>(long: &Bound<'_, PyInt>) -> PyResult<Vec<u
}
let n_bytes =
unsafe { ffi::PyLong_AsNativeBytes(long.as_ptr().cast(), core::ptr::null_mut(), 0, flags) };
let n_bytes_unsigned: usize = n_bytes
.try_into()
.map_err(|_| crate::PyErr::fetch(long.py()))?;
let n_bytes_unsigned: usize = n_bytes.try_into().map_err(|_| PyErr::fetch(long.py()))?;
if n_bytes == 0 {
return Ok(buffer);
}
Expand Down Expand Up @@ -430,6 +607,41 @@ class C:
})
}

#[cfg(all(not(Py_LIMITED_API), Py_3_14))]
#[test]
fn pylong_export() {
Python::attach(|py| {
if !is_30bit_layout() {
return;
}

let small = 42_i32.into_pyobject(py).unwrap();
let (negative, compact, digits) =
pylong_visit_digits(small.as_any().as_borrowed(), |negative, compact, digits| {
Ok((negative, compact, digits.map(<[u32]>::to_vec)))
})
.unwrap();
assert!(!negative);
assert_eq!(compact, 42);
assert_eq!(digits, None);

let big = BigInt::new(Sign::Minus, vec![u32::MAX, 0x8000_0001, 0x1234_5678, 1])
.into_pyobject(py)
.unwrap();
let (negative, digits) =
pylong_visit_digits(big.as_any().as_borrowed(), |negative, _, digits| {
Ok((negative, digits.map(<[u32]>::to_vec)))
})
.unwrap();
assert!(negative);
let digits = digits.unwrap();
assert_eq!(
int_from_pylong_digits(&digits),
vec![u32::MAX, 0x8000_0001, 0x1234_5678, 1]
);
});
}

/// `OverflowError` on converting Python int to BigInt, see issue #629
#[test]
fn check_overflow() {
Expand Down Expand Up @@ -462,10 +674,28 @@ class C:
});
}

#[cfg(all(not(Py_LIMITED_API), Py_3_14))]
#[test]
fn biguint_negative() {
Python::attach(|py| {
if !is_30bit_layout() {
return;
}

Comment thread
chirizxc marked this conversation as resolved.
Outdated
let value = py.eval(c"-(1 << 130)", None, None).unwrap();
let err = value.extract::<BigUint>().unwrap_err();
assert!(err.is_instance_of::<crate::exceptions::PyValueError>(py));
assert_eq!(
err.value(py).to_string(),
"can't convert negative int to unsigned"
);
});
}

#[test]
fn from_py_float_type_error() {
Python::attach(|py| {
let obj = (12.3f64).into_pyobject(py).unwrap();
let obj = 12.3f64.into_pyobject(py).unwrap();
let err = obj.extract::<BigInt>().unwrap_err();
assert!(err.is_instance_of::<PyTypeError>(py));

Expand Down
6 changes: 3 additions & 3 deletions src/conversions/std/num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,11 +393,11 @@ impl Drop for ExportGuard {
// Builds an int from an iterator of 30-bit digits
#[cfg(all(Py_3_14, not(Py_LIMITED_API)))]
#[inline]
pub(crate) fn pylong_from_digits<'py, I: ExactSizeIterator<Item = u32>>(
py: Python<'py>,
pub(crate) fn pylong_from_digits<I: ExactSizeIterator<Item = u32>>(
py: Python<'_>,
negative: bool,
digits: I,
) -> Bound<'py, PyInt> {
) -> Bound<'_, PyInt> {
let digits_len = digits.len();
let mut ptr = core::ptr::null_mut();
let writer = unsafe {
Expand Down
Loading