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.

Sign up for GitHub

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

Jump to bottom

Fix visibility of get_limbs_representations #139

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix visibility of get_limbs_representations #139

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eb2c255
Fix visibility of `get_limbs_representations`
skaunov Feb 8, 2024
78c4820
Visibility doc update, tiny style fixes
skaunov Feb 9, 2024
ae57a78
Update src/fields/emulated_fp/allocated_field_var.rs
Pratyush Mar 19, 2024
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
16 changes: 8 additions & 8 deletions src/fields/emulated_fp/allocated_field_var.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,10 +302,10 @@ impl<TargetF: PrimeField, BaseF: PrimeField> AllocatedEmulatedFpVar<TargetF, Bas
Ok(inverse)
}

/// Convert a `TargetF` element into limbs (not constraints)
/// Convert a `TargetF` element into limbs (not constraints).
/// This is an internal function that would be reused by a number of other
/// functions
pub fn get_limbs_representations(
/// functions.
pub(super) fn get_limbs_representations(
Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine to leave this public, as possibly downstream code might need to use it. Maybe we can mark it as #[doc(hidden)] instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have code that uses it, don't see why it has to be hidden even.

elem: &TargetF,
optimization_type: OptimizationType,
) -> R1CSResult<Vec<BaseF>> {
Expand Down Expand Up @@ -464,7 +464,7 @@ impl<TargetF: PrimeField, BaseF: PrimeField> AllocatedEmulatedFpVar<TargetF, Bas
for limb in p_representations.iter() {
p_gadget_limbs.push(FpVar::<BaseF>::Constant(*limb));
}
let p_gadget = AllocatedEmulatedFpVar::<TargetF, BaseF> {
let p_gadget = Self {
cs: self.cs(),
limbs: p_gadget_limbs,
num_of_additions_over_normal_form: BaseF::one(),
Expand Down Expand Up @@ -621,10 +621,10 @@ impl<TargetF: PrimeField, BaseF: PrimeField> AllocatedEmulatedFpVar<TargetF, Bas
}

/// Allocates a new non-native field witness with value given by the
/// function `f`. Enforces that the field element has value in `[0, modulus)`,
/// and returns the bits of its binary representation.
/// The bits are in little-endian (i.e., the bit at index 0 is the LSB) and the
/// bit-vector is empty in non-witness allocation modes.
/// function `f`. Enforces that the field element has value
/// in `[0, modulus)`, and returns the bits of its binary representation.
/// The bits are in little-endian (i.e., the bit at index 0 is the LSB) and
/// the bit-vector is empty in non-witness allocation modes.
pub fn new_witness_with_le_bits<T: Borrow<TargetF>>(
cs: impl Into<Namespace<BaseF>>,
f: impl FnOnce() -> Result<T, SynthesisError>,
Expand Down