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

Add to_slice and to_vec functions #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

uzytkownik
Copy link

I needed to convert to/from mpz_t. While converting into is relativly simple due to from_slice function the invert requires allocation of array with base 28 making it less efficient (requiring 2 copies and 2 convertions from/to 8 bit instead of 1 or 0). Those functions would allow inspecting representation of function without need of allocating and creating additional copies.

@cuviper
Copy link
Member

cuviper commented Jan 7, 2018

I hesitate to expose the internal representation so much, for the borrowed slice especially.

How would you feel about some kind of iterator instead? We could make it return bytes or even larger units, without having to allocate the full conversion.

@uzytkownik
Copy link
Author

uzytkownik commented Jan 7, 2018

That would not solve the problem fully due to mpz_export/mpz_import functions. GMP documents internals only partially (namely - there are some compile-time constants) so I guess it should work assuming some support for nails,

@cuviper
Copy link
Member

cuviper commented Mar 4, 2018

Perhaps we could frame this in a way that doesn't commit to a particular representation? Like:

impl BigUint {
    // Returns `(count, order, size, endian, nails, ptr)` in the style of `mpz_import`
    fn export_raw<F, T>(&self, f: F) -> (usize, c_int, usize, c_int, usize, *const c_void);
}

Obviously that's tailored directly for GMP, but I'm not sure how to generalize this idea.

@tspiteri
Copy link
Contributor

tspiteri commented Mar 18, 2018

If what is required is conversion to mpz_t, an iterator could work, considering that GMP does document its internal representation. As a note, I don't think nails need to be supported, considering that if I'm not mistaken, compiling GMP with nails has been broken since late in the 4.* series (that's ancient). I hacked up something assuming that BigUint has some digits() method that gives an iterator with the digits of our requested size (u32 or u64), least significant first. I did not test it, this is just to give an idea of how to go about it:

use gmp_mpfr_sys::gmp;
use std::os::raw::c_int;
use std::slice;
fn copy_to_gmp(src: &BigUint, dst: &mut gmp::mpz_t) {
    assert_eq!(gmp::NAIL_BITS, 0, "nails not supported");
    let bits = src.bits();
    dst.size = 0;
    if bits == 0 {
        return;
    }
    let limb_bits = gmp::LIMB_BITS as usize;
    let limb_count = (bits + limb_bits - 1) / limb_bits;
    let limbs = unsafe {
        gmp::_mpz_realloc(mpz_t, limb_count);
        slice::from_raw_parts_mut(dst.d, limb_count)
    };
    // limb_t is either u32 or u64
    let mut digits = src.digits::<gmp::limb_t>();
    for i in limbs.iter_mut() {
        *i = digits.next().expect("not enough digits");
    }
    assert!(digits.next().is_none(), "too many digits");
    dst.size = limb_count as c_int;
    assert!(dst.size > 0 && dst.size as usize == limb_count, "overflow");
}

@dignifiedquire
Copy link
Contributor

Another data point: it would be great to have the limbs exposed as an iterator

@Speedy37
Copy link
Contributor

I've the same issue, I need to access the internal &[u32] slice.
There is a from_slice, so a to_slice function returning a Vec would be nice and doesn't require to commit to a predefined internal representation.

For performance, it would be nice to have an iterable version of it or a non documented version (like as_slice).

@expenses
Copy link

I'd like either a function that returned the underlying slice or an iterator too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants