Skip to content

Add unsafe Setters #491

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

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
36 changes: 34 additions & 2 deletions src/integer/mat_poly_over_z/unsafe_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,16 @@
//! [FLINT](https://flintlib.org/) structs. Therefore, they require to be unsafe.

use super::MatPolyOverZ;
use crate::macros::unsafe_passthrough::unsafe_getter;
use flint_sys::fmpz_poly_mat::fmpz_poly_mat_struct;
use crate::macros::unsafe_passthrough::{unsafe_getter, unsafe_setter};
use flint_sys::fmpz_poly_mat::{fmpz_poly_mat_clear, fmpz_poly_mat_struct};

unsafe_getter!(MatPolyOverZ, matrix, fmpz_poly_mat_struct);
unsafe_setter!(
MatPolyOverZ,
matrix,
fmpz_poly_mat_struct,
fmpz_poly_mat_clear
);

#[cfg(test)]
mod test_get_fmpz_poly_mat_struct {
Expand All @@ -40,3 +46,29 @@ mod test_get_fmpz_poly_mat_struct {
assert_eq!(poly, mat.get_entry(0, 0).unwrap());
}
}

#[cfg(test)]
mod test_set_fmpz_poly_mat_struct {
use super::MatPolyOverZ;
use crate::{integer::PolyOverZ, traits::MatrixGetEntry};
use flint_sys::fmpz_poly_mat::fmpz_poly_mat_init;
use std::{mem::MaybeUninit, str::FromStr};

/// Checks availability of the setter for [`MatPolyOverZ::matrix`]
/// and its ability to modify [`MatPolyOverZ`].
#[test]
#[allow(unused_mut)]
fn availability_and_modification() {
let mut mat = MatPolyOverZ::from_str("[[1 1]]").unwrap();
let mut flint_struct = MaybeUninit::uninit();
let flint_struct = unsafe {
fmpz_poly_mat_init(flint_struct.as_mut_ptr(), 1, 1);
flint_struct.assume_init()
};
let poly = PolyOverZ::default();

unsafe { mat.set_fmpz_poly_mat_struct(flint_struct) };

assert_eq!(poly, mat.get_entry(0, 0).unwrap());
}
}
30 changes: 28 additions & 2 deletions src/integer/mat_z/unsafe_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@
//! [FLINT](https://flintlib.org/) structs. Therefore, they require to be unsafe.

use super::MatZ;
use crate::macros::unsafe_passthrough::unsafe_getter;
use flint_sys::fmpz_mat::fmpz_mat_struct;
use crate::macros::unsafe_passthrough::{unsafe_getter, unsafe_setter};
use flint_sys::fmpz_mat::{fmpz_mat_clear, fmpz_mat_struct};

unsafe_getter!(MatZ, matrix, fmpz_mat_struct);
unsafe_setter!(MatZ, matrix, fmpz_mat_struct, fmpz_mat_clear);

#[cfg(test)]
mod test_get_fmpz_mat_struct {
Expand Down Expand Up @@ -42,3 +43,28 @@ mod test_get_fmpz_mat_struct {
assert_eq!(Z::from(2), mat.get_entry(0, 0).unwrap());
}
}

#[cfg(test)]
mod test_set_fmpz_mat_struct {
use super::MatZ;
use crate::{integer::Z, traits::MatrixGetEntry};
use flint_sys::fmpz_mat::fmpz_mat_init;
use std::{mem::MaybeUninit, str::FromStr};

/// Checks availability of the setter for [`MatZ::matrix`]
/// and its ability to modify [`MatZ`].
#[test]
#[allow(unused_mut)]
fn availability_and_modification() {
let mut mat = MatZ::from_str("[[1]]").unwrap();
let mut flint_struct = MaybeUninit::uninit();
let flint_struct = unsafe {
fmpz_mat_init(flint_struct.as_mut_ptr(), 1, 1);
flint_struct.assume_init()
};

unsafe { mat.set_fmpz_mat_struct(flint_struct) };

assert_eq!(Z::from(0), mat.get_entry(0, 0).unwrap());
}
}
29 changes: 27 additions & 2 deletions src/integer/poly_over_z/unsafe_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@
//! [FLINT](https://flintlib.org/) structs. Therefore, they require to be unsafe.

use super::PolyOverZ;
use crate::macros::unsafe_passthrough::unsafe_getter;
use flint_sys::fmpz_poly::fmpz_poly_struct;
use crate::macros::unsafe_passthrough::{unsafe_getter, unsafe_setter};
use flint_sys::fmpz_poly::{fmpz_poly_clear, fmpz_poly_struct};

unsafe_getter!(PolyOverZ, poly, fmpz_poly_struct);
unsafe_setter!(PolyOverZ, poly, fmpz_poly_struct, fmpz_poly_clear);

#[cfg(test)]
mod test_get_fmpz_poly_struct {
Expand All @@ -34,3 +35,27 @@ mod test_get_fmpz_poly_struct {
assert_eq!(PolyOverZ::from(2), poly);
}
}

#[cfg(test)]
mod test_set_fmpz_poly_struct {
use super::PolyOverZ;
use flint_sys::fmpz_poly::fmpz_poly_init;
use std::mem::MaybeUninit;

/// Checks availability of the setter for [`PolyOverZ::poly`]
/// and its ability to modify [`PolyOverZ`].
#[test]
#[allow(unused_mut)]
fn availability_and_modification() {
let mut poly = PolyOverZ::from(1);
let mut flint_struct = MaybeUninit::uninit();
let flint_struct = unsafe {
fmpz_poly_init(flint_struct.as_mut_ptr());
flint_struct.assume_init()
};

unsafe { poly.set_fmpz_poly_struct(flint_struct) };

assert_eq!(PolyOverZ::default(), poly);
}
}
29 changes: 26 additions & 3 deletions src/integer/z/unsafe_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,20 @@
//! This module contains public functions that enable access to underlying
//! [FLINT](https://flintlib.org/) structs. Therefore, they require to be unsafe.

use crate::{integer::Z, macros::unsafe_passthrough::unsafe_getter};
use flint_sys::fmpz::fmpz;
use crate::{
integer::Z,
macros::unsafe_passthrough::{unsafe_getter, unsafe_setter},
};
use flint_sys::fmpz::{fmpz, fmpz_clear};

unsafe_getter!(Z, value, fmpz);
unsafe_setter!(Z, value, fmpz, fmpz_clear);

#[cfg(test)]
mod test_get_fmpz {
use super::Z;

/// Checks availability of the getter for [`Z::value`]
/// Checks availability of the setter for [`Z::value`]
Copy link
Member

Choose a reason for hiding this comment

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

But you check the getter here. So the initial comment should be correct

/// and its ability to be modified.
#[test]
#[allow(unused_mut)]
Expand All @@ -32,3 +36,22 @@ mod test_get_fmpz {
assert_eq!(Z::from(2), integer);
}
}

#[cfg(test)]
mod test_set_fmpz {
use super::Z;
use flint_sys::fmpz::fmpz;

/// Checks availability of the setter for [`Z::value`]
/// and its ability to modify [`Z`].
#[test]
#[allow(unused_mut)]
fn availability_and_modification() {
let mut integer = Z::from(1);
let b = fmpz(2);

unsafe { integer.set_fmpz(b) };

assert_eq!(Z::from(2), integer);
}
}
15 changes: 14 additions & 1 deletion src/integer_mod_q/mat_polynomial_ring_zq/unsafe_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
//! [FLINT](https://flintlib.org/) structs. Therefore, they require to be unsafe.

use super::MatPolynomialRingZq;
use crate::macros::unsafe_passthrough::unsafe_getter_indirect;
use crate::macros::unsafe_passthrough::{unsafe_getter_indirect, unsafe_setter_indirect};
use flint_sys::{fmpz_poly_mat::fmpz_poly_mat_struct, fq::fq_ctx_struct};

unsafe_getter_indirect!(
Expand All @@ -25,3 +25,16 @@ unsafe_getter_indirect!(
get_fq_ctx_struct,
fq_ctx_struct
);

unsafe_setter_indirect!(
MatPolynomialRingZq,
matrix,
set_fmpz_poly_mat_struct,
fmpz_poly_mat_struct
);
unsafe_setter_indirect!(
MatPolynomialRingZq,
modulus,
set_fq_ctx_struct,
fq_ctx_struct
);
37 changes: 35 additions & 2 deletions src/integer_mod_q/mat_zq/unsafe_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,20 @@
//! [FLINT](https://flintlib.org/) structs. Therefore, they require to be unsafe.

use super::MatZq;
use crate::macros::unsafe_passthrough::{unsafe_getter, unsafe_getter_indirect};
use flint_sys::{fmpz_mod::fmpz_mod_ctx, fmpz_mod_mat::fmpz_mod_mat_struct};
use crate::macros::unsafe_passthrough::{
unsafe_getter, unsafe_getter_indirect, unsafe_setter, unsafe_setter_indirect,
};
use flint_sys::{
fmpz_mod::fmpz_mod_ctx,
fmpz_mod_mat::{fmpz_mod_mat_clear, fmpz_mod_mat_struct},
};

unsafe_getter!(MatZq, matrix, fmpz_mod_mat_struct);
unsafe_getter_indirect!(MatZq, modulus, get_fmpz_mod_ctx, fmpz_mod_ctx);

unsafe_setter!(MatZq, matrix, fmpz_mod_mat_struct, fmpz_mod_mat_clear);
unsafe_setter_indirect!(MatZq, modulus, set_fmpz_mod_ctx, fmpz_mod_ctx);

#[cfg(test)]
mod test_get_fmpz_mod_mat_struct {
use super::MatZq;
Expand All @@ -35,3 +43,28 @@ mod test_get_fmpz_mod_mat_struct {
assert_eq!(MatZq::from_str("[[3]] mod 5").unwrap(), mat);
}
}

#[cfg(test)]
mod test_set_fmpz_mod_mat_struct {
use super::MatZq;
use flint_sys::{fmpz::fmpz, fmpz_mod_mat::fmpz_mod_mat_init};
use std::{mem::MaybeUninit, str::FromStr};

/// Checks availability of the setter for [`MatZq::matrix`]
/// and its ability to modify [`MatZq`].
#[test]
fn availability_and_modification() {
let mut mat = MatZq::from_str("[[3]] mod 7").unwrap();
let mut flint_struct = MaybeUninit::uninit();
let flint_struct = unsafe {
fmpz_mod_mat_init(flint_struct.as_mut_ptr(), 1, 1, &fmpz(7));
flint_struct.assume_init()
};

unsafe {
mat.set_fmpz_mod_mat_struct(flint_struct);
};

assert_eq!(MatZq::new(1, 1, 7), mat);
}
}
69 changes: 68 additions & 1 deletion src/integer_mod_q/modulus/unsafe_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,52 @@

use super::Modulus;
use crate::macros::unsafe_passthrough::unsafe_getter_mod;
use flint_sys::fmpz_mod::fmpz_mod_ctx;
use flint_sys::fmpz_mod::{fmpz_mod_ctx, fmpz_mod_ctx_clear};

unsafe_getter_mod!(Modulus, modulus, fmpz_mod_ctx);

impl Modulus {
/// Returns a mutable reference to the field `modulus` of type [`Modulus`].
Copy link
Member

Choose a reason for hiding this comment

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

The function returns nothing. It sets the value

Suggested change
/// Returns a mutable reference to the field `modulus` of type [`Modulus`].
/// Sets a mutable reference to the field `modulus` of type [`Modulus`] to a given `fmpz_mod_ctx`.

///
/// Parameters:
/// - `flint_struct`: value to set the attribute to
///
/// **WARNING:** The returned struct is part of [`flint_sys`].
/// Any changes to this object are unsafe and may introduce memory leaks.
/// Please be aware that most moduli are shared across multiple instances and all
/// modifications of this struct will affect any other instance with a reference to this object.
Comment on lines +24 to +27
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// **WARNING:** The returned struct is part of [`flint_sys`].
/// Any changes to this object are unsafe and may introduce memory leaks.
/// Please be aware that most moduli are shared across multiple instances and all
/// modifications of this struct will affect any other instance with a reference to this object.
/// **WARNING:** The set struct is part of [`flint_sys`].
/// Any changes to this object are unsafe and may introduce memory leaks.
/// Please be aware that most moduli are shared across multiple instances and all
/// modifications of this struct will affect any other instance with a reference to this object.

///
/// This function is a passthrough to enable users of this library to use [`flint_sys`]
/// and with that [FLINT](https://flintlib.org/) functions that might not be covered in our library yet.
/// If this is the case, please consider contributing to this open-source project
/// by opening a Pull Request at [qfall_math](https://github.com/qfall/math)
/// to provide this feature in the future.
///
/// # Safety
/// Ensure that the old `modulus` does not share any memory with any moduli
/// that might be used in the future. The memory of the old `modulus` is freed
/// using this function.
///
/// Any [`flint_sys`] struct and function is part of a FFI to the C-library `FLINT`.
/// As `FLINT` is a C-library, it does not provide all memory safety features
/// that Rust and our Wrapper provide.
/// Thus, using functions of [`flint_sys`] can introduce memory leaks.
pub unsafe fn set_fmpz_mod_ctx(&mut self, flint_struct: fmpz_mod_ctx) {
let modulus = std::rc::Rc::<fmpz_mod_ctx>::get_mut(&mut self.modulus).unwrap();

// free memory of old modulus before new values of modulus are copied
unsafe { fmpz_mod_ctx_clear(modulus) };

modulus.add_fxn = flint_struct.add_fxn;
modulus.mod_ = flint_struct.mod_;
modulus.mul_fxn = flint_struct.mul_fxn;
modulus.n = flint_struct.n;
modulus.n_limbs = flint_struct.n_limbs;
modulus.ninv_limbs = flint_struct.ninv_limbs;
modulus.sub_fxn = flint_struct.sub_fxn;
}
}

#[cfg(test)]
mod test_get_fmpz_mod_ctx {
use super::Modulus;
Expand All @@ -33,3 +75,28 @@ mod test_get_fmpz_mod_ctx {
assert_eq!(Modulus::from(2), modulus);
}
}

#[cfg(test)]
mod test_set_fmpz_mod_ctx {
use super::Modulus;
use flint_sys::{fmpz::fmpz, fmpz_mod::fmpz_mod_ctx_init};
use std::mem::MaybeUninit;

/// Checks availability of the setter for [`Modulus::modulus`]
/// and its ability to modify [`Modulus`].
#[test]
#[allow(unused_mut)]
fn availability_and_modification() {
let mut modulus = Modulus::from(3);

let mut flint_struct = MaybeUninit::uninit();
let mut flint_struct = unsafe {
fmpz_mod_ctx_init(flint_struct.as_mut_ptr(), &fmpz(2));
flint_struct.assume_init()
};

unsafe { modulus.set_fmpz_mod_ctx(flint_struct) };

assert_eq!(Modulus::from(2), modulus);
}
}
Comment on lines +79 to +102
Copy link
Member

Choose a reason for hiding this comment

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

How does it work for multiple references?
You say that the function only works if no other references exist/are used in the comment. But is there no way to rewrite the function such that it does? Or is it just too much for us to implement right now?
I tried this, but it did not work:

/// Checks if setter works for all references.

#[test]
#[allow(unused_mut)]
fn counter_works() {
    let mut modulus_1 = Modulus::from(3);
    let mut modulus_2 = Modulus::from(3);
    let mut modulus_3 = modulus_1.clone();

    let mut flint_struct = MaybeUninit::uninit();
    let mut flint_struct = unsafe {
        fmpz_mod_ctx_init(flint_struct.as_mut_ptr(), &fmpz(2));
        flint_struct.assume_init()
    };

    unsafe { modulus_1.set_fmpz_mod_ctx(flint_struct) };

    assert_eq!(Modulus::from(2), modulus_1);
    assert_eq!(Modulus::from(3), modulus_2);
    assert_eq!(Modulus::from(2), modulus_3);
}

Loading