Skip to content

Commit

Permalink
fix: Allow array map on empty arrays (noir-lang/noir#6305)
Browse files Browse the repository at this point in the history
fix: Display function name and body when inlining recursion limit hit (noir-lang/noir#6291)
feat(interpreter): Comptime derive generators (noir-lang/noir#6303)
fix: enforce correctness of decompositions performed at compile time (noir-lang/noir#6278)
feat: Warn about private types leaking in public functions and struct fields (noir-lang/noir#6296)
chore(docs): refactoring guides and some other nits (noir-lang/noir#6175)
fix: Do not warn on unused self in traits (noir-lang/noir#6298)
fix: Reject invalid expression with in CLI parser (noir-lang/noir#6287)
  • Loading branch information
AztecBot committed Oct 22, 2024
2 parents c4322da + 7a19f7a commit 9c91307
Show file tree
Hide file tree
Showing 13 changed files with 213 additions and 22 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
67ac0d60c3e8b450a9e871f3edb29322ac5045d2
51ae1b324cd73fdb4fe3695b5d483a44b4aff4a9
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ pub(super) fn simplify_call(
} else {
unreachable!("ICE: Intrinsic::ToRadix return type must be array")
};
let result_array = constant_to_radix(endian, field, 2, limb_count, dfg);

SimplifyResult::SimplifiedTo(result_array)
constant_to_radix(endian, field, 2, limb_count, dfg)
} else {
SimplifyResult::None
}
Expand All @@ -79,10 +77,7 @@ pub(super) fn simplify_call(
} else {
unreachable!("ICE: Intrinsic::ToRadix return type must be array")
};

let result_array = constant_to_radix(endian, field, radix, limb_count, dfg);

SimplifyResult::SimplifiedTo(result_array)
constant_to_radix(endian, field, radix, limb_count, dfg)
} else {
SimplifyResult::None
}
Expand Down Expand Up @@ -611,22 +606,29 @@ fn constant_to_radix(
radix: u32,
limb_count: u32,
dfg: &mut DataFlowGraph,
) -> ValueId {
) -> SimplifyResult {
let bit_size = u32::BITS - (radix - 1).leading_zeros();
let radix_big = BigUint::from(radix);
assert_eq!(BigUint::from(2u128).pow(bit_size), radix_big, "ICE: Radix must be a power of 2");
let big_integer = BigUint::from_bytes_be(&field.to_be_bytes());

// Decompose the integer into its radix digits in little endian form.
let decomposed_integer = big_integer.to_radix_le(radix);
let mut limbs = vecmap(0..limb_count, |i| match decomposed_integer.get(i as usize) {
Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]),
None => FieldElement::zero(),
});
if endian == Endian::Big {
limbs.reverse();
if limb_count < decomposed_integer.len() as u32 {
// `field` cannot be represented as `limb_count` bits.
// defer error to acir_gen.
SimplifyResult::None
} else {
let mut limbs = vecmap(0..limb_count, |i| match decomposed_integer.get(i as usize) {
Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]),
None => FieldElement::zero(),
});
if endian == Endian::Big {
limbs.reverse();
}
let result_array = make_constant_array(dfg, limbs, Type::unsigned(bit_size));
SimplifyResult::SimplifiedTo(result_array)
}
make_constant_array(dfg, limbs, Type::unsigned(bit_size))
}

fn to_u8_vec(dfg: &DataFlowGraph, values: im::Vector<Id<Value>>) -> Vec<u8> {
Expand Down
29 changes: 27 additions & 2 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,13 +291,14 @@ impl InlineContext {
) -> Vec<ValueId> {
self.recursion_level += 1;

let source_function = &ssa.functions[&id];

if self.recursion_level > RECURSION_LIMIT {
panic!(
"Attempted to recur more than {RECURSION_LIMIT} times during function inlining."
"Attempted to recur more than {RECURSION_LIMIT} times during inlining function '{}': {}", source_function.name(), source_function
);
}

let source_function = &ssa.functions[&id];
let mut context = PerFunctionContext::new(self, source_function);

let parameters = source_function.parameters();
Expand Down Expand Up @@ -967,4 +968,28 @@ mod test {
let main = ssa.main();
assert_eq!(main.reachable_blocks().len(), 4);
}

#[test]
#[should_panic(
expected = "Attempted to recur more than 1000 times during inlining function 'main': acir(inline) fn main f0 {"
)]
fn unconditional_recursion() {
// fn main f1 {
// b0():
// call f1()
// return
// }
let main_id = Id::test_new(0);
let mut builder = FunctionBuilder::new("main".into(), main_id);

let main = builder.import_function(main_id);
let results = builder.insert_call(main, Vec::new(), vec![]).to_vec();
builder.terminate_with_return(results);

let ssa = builder.finish();
assert_eq!(ssa.functions.len(), 1);

let inlined = ssa.inline_functions();
assert_eq!(inlined.functions.len(), 0);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,10 @@ pub enum InterpreterError {
location: Location,
expression: String,
},
UnknownArrayLength {
length: Type,
location: Location,
},

// These cases are not errors, they are just used to prevent us from running more code
// until the loop can be resumed properly. These cases will never be displayed to users.
Expand Down Expand Up @@ -299,7 +303,8 @@ impl InterpreterError {
| InterpreterError::DuplicateGeneric { duplicate_location: location, .. }
| InterpreterError::TypeAnnotationsNeededForMethodCall { location }
| InterpreterError::CannotResolveExpression { location, .. }
| InterpreterError::CannotSetFunctionBody { location, .. } => *location,
| InterpreterError::CannotSetFunctionBody { location, .. }
| InterpreterError::UnknownArrayLength { location, .. } => *location,

InterpreterError::FailedToParseMacro { error, file, .. } => {
Location::new(error.span(), *file)
Expand Down Expand Up @@ -635,6 +640,10 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic {
let msg = format!("`{expression}` is not a valid function body");
CustomDiagnostic::simple_error(msg, String::new(), location.span)
}
InterpreterError::UnknownArrayLength { length, location } => {
let msg = format!("Could not determine array length `{length}`");
CustomDiagnostic::simple_error(msg, String::new(), location.span)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ impl<'local, 'context> Interpreter<'local, 'context> {
"as_slice" => as_slice(interner, arguments, location),
"ctstring_eq" => ctstring_eq(arguments, location),
"ctstring_hash" => ctstring_hash(arguments, location),
"derive_pedersen_generators" => {
derive_generators(interner, arguments, return_type, location)
}
"expr_as_array" => expr_as_array(interner, arguments, return_type, location),
"expr_as_assert" => expr_as_assert(interner, arguments, return_type, location),
"expr_as_assert_eq" => expr_as_assert_eq(interner, arguments, return_type, location),
Expand Down Expand Up @@ -2770,3 +2773,56 @@ fn ctstring_eq(arguments: Vec<(Value, Location)>, location: Location) -> IResult
fn ctstring_hash(arguments: Vec<(Value, Location)>, location: Location) -> IResult<Value> {
hash_item(arguments, location, get_ctstring)
}

fn derive_generators(
interner: &mut NodeInterner,
arguments: Vec<(Value, Location)>,
return_type: Type,
location: Location,
) -> IResult<Value> {
let (domain_separator_string, starting_index) = check_two_arguments(arguments, location)?;

let domain_separator_location = domain_separator_string.1;
let (domain_separator_string, _) = get_array(interner, domain_separator_string)?;
let starting_index = get_u32(starting_index)?;

let domain_separator_string =
try_vecmap(domain_separator_string, |byte| get_u8((byte, domain_separator_location)))?;

let (size, elements) = match return_type.clone() {
Type::Array(size, elements) => (size, elements),
_ => panic!("ICE: Should only have an array return type"),
};

let Some(num_generators) = size.evaluate_to_u32() else {
return Err(InterpreterError::UnknownArrayLength { length: *size, location });
};

let generators = bn254_blackbox_solver::derive_generators(
&domain_separator_string,
num_generators,
starting_index,
);

let is_infinite = FieldElement::zero();
let x_field_name: Rc<String> = Rc::new("x".to_owned());
let y_field_name: Rc<String> = Rc::new("y".to_owned());
let is_infinite_field_name: Rc<String> = Rc::new("is_infinite".to_owned());
let mut results = Vector::new();
for gen in generators {
let x_big: BigUint = gen.x.into();
let x = FieldElement::from_be_bytes_reduce(&x_big.to_bytes_be());
let y_big: BigUint = gen.y.into();
let y = FieldElement::from_be_bytes_reduce(&y_big.to_bytes_be());
let mut embedded_curve_point_fields = HashMap::default();
embedded_curve_point_fields.insert(x_field_name.clone(), Value::Field(x));
embedded_curve_point_fields.insert(y_field_name.clone(), Value::Field(y));
embedded_curve_point_fields
.insert(is_infinite_field_name.clone(), Value::Field(is_infinite));
let embedded_curve_point_struct =
Value::Struct(embedded_curve_point_fields, *elements.clone());
results.push_back(embedded_curve_point_struct);
}

Ok(Value::Array(results, return_type))
}
2 changes: 2 additions & 0 deletions noir/noir-repo/docs/docs/noir/concepts/data_types/arrays.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ fn main() {

Same as fold, but uses the first element as the starting element.

Requires `self` to be non-empty.

```rust
fn reduce(self, f: fn(T, T) -> T) -> T
```
Expand Down
15 changes: 12 additions & 3 deletions noir/noir-repo/noir_stdlib/src/array/mod.nr
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ impl<T, let N: u32> [T; N] {
/// assert_eq(b, [2, 4, 6]);
/// ```
pub fn map<U, Env>(self, f: fn[Env](T) -> U) -> [U; N] {
let first_elem = f(self[0]);
let mut ret = [first_elem; N];
let uninitialized = crate::mem::zeroed();
let mut ret = [uninitialized; N];

for i in 1..self.len() {
for i in 0..self.len() {
ret[i] = f(self[i]);
}

Expand Down Expand Up @@ -79,6 +79,8 @@ impl<T, let N: u32> [T; N] {
}

/// Same as fold, but uses the first element as the starting element.
///
/// Requires the input array to be non-empty.
///
/// Example:
///
Expand Down Expand Up @@ -217,3 +219,10 @@ impl<let N: u32> From<str<N>> for [u8; N] {
s.as_bytes()
}
}

mod test {
#[test]
fn map_empty() {
assert_eq([].map(|x| x + 1), []);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "comptime_derive_generators"
type = "bin"
authors = [""]
compiler_version = ">=0.35.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
use std::embedded_curve_ops::EmbeddedCurvePoint;

fn main() {
comptime
{
// Result computed from executing `derive_generators` with non-comptime Noir
let result = [
EmbeddedCurvePoint {
x: 0x0224a8abc6c8b8d50373d64cd2a1ab1567bf372b3b1f7b861d7f01257052d383,
y: 0x2358629b90eafb299d6650a311e79914b0215eb0a790810b26da5a826726d711,
is_infinite: false
}, EmbeddedCurvePoint {
x: 0x0f106f6d46bc904a5290542490b2f238775ff3c445b2f8f704c466655f460a2a,
y: 0x29ab84d472f1d33f42fe09c47b8f7710f01920d6155250126731e486877bcf27,
is_infinite: false
}, EmbeddedCurvePoint {
x: 0x0298f2e42249f0519c8a8abd91567ebe016e480f219b8c19461d6a595cc33696,
y: 0x035bec4b8520a4ece27bd5aafabee3dfe1390d7439c419a8c55aceb207aac83b,
is_infinite: false
}, EmbeddedCurvePoint {
x: 0x2c9628479de4181ea77e7b0913ccf41d2a74155b1d9c82eaa220c218781f6f3b,
y: 0x278f86b8fd95520b5da23bee1a5e354dc5dcb0cb43d6b76e628ddbffb101d776,
is_infinite: false
}, EmbeddedCurvePoint {
x: 0x0be1916f382e3532aa53a766fe74b1a983784caab90290aea7bf616bc371fb41,
y: 0x0f65545005e896f14249956344faf9addd762b7573a487b58f805a361d920a20,
is_infinite: false
}, EmbeddedCurvePoint {
x: 0x29ff8437ae5bec89981441b23036a22b7fd5bee9eff0e83c0dd5b87bfb5bd60e,
y: 0x1fd247352b77e2676b22db23cf7cd482474f543e3480b5a39c42f839a306be10,
is_infinite: false
}, EmbeddedCurvePoint {
x: 0x2f3bd4e98f8c8458cd58888749f0f5e582a43565767398e08e50e94b9b19a4d9,
y: 0x1f534906d1aa8b4ba74ad9e3f85ae3f8295e51eaafd15b5d116801b96360205b,
is_infinite: false
}, EmbeddedCurvePoint {
x: 0x27759098f425b76447c2c52728576803a1ac5de37bba875ac47cdcff539ab931,
y: 0x0aa47ee64d12d856cfb81b595c1d60ceecb693f0fdae644746ff333e39f61db7,
is_infinite: false
}, EmbeddedCurvePoint {
x: 0x015ca8d68616fde86c9108e3db04f588e0f308e60d367e963b7d460fe9a65e6c,
y: 0x2cf918009dda942ac9d59903cd2d0294d8738f938b1394170d892a027d0f347b,
is_infinite: false
}, EmbeddedCurvePoint {
x: 0x0d1783d5b256765515f3c9988df9f1ba7e6f5fb0248c8971fbc503ffd5187714,
y: 0x2ebb434ff4857fc3621f3bc3c6b8002b17d02d9c204e75f19b8f0b99ea68402c,
is_infinite: false
}
];

let generators: [EmbeddedCurvePoint; 10] = std::hash::derive_generators("DEFAULT_DOMAIN_SEPARATOR".as_bytes(), 5);

for i in 0..10 {
assert(generators[i].x == result[i].x);
assert(generators[i].y == result[i].y);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "invalid_comptime_bits_decomposition"
type = "bin"
authors = [""]
compiler_version = ">=0.30.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
fn main() -> pub [u1; 1] {
let large_number: Field = 2;

large_number.to_be_bits()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "invalid_comptime_bytes_decomposition"
type = "bin"
authors = [""]
compiler_version = ">=0.30.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
fn main() -> pub [u8; 1] {
let large_number: Field = 256;
large_number.to_be_bytes()
}

0 comments on commit 9c91307

Please sign in to comment.