From 601fd9afc502236af1db0c4492698ba2298c7501 Mon Sep 17 00:00:00 2001 From: jfecher Date: Thu, 22 Feb 2024 15:31:36 +0000 Subject: [PATCH] fix!: Ban Fields in for loop indices and bitwise ops (#4376) # Description ## Problem\* Resolves https://github.com/noir-lang/noir/issues/3639 Resolves https://github.com/noir-lang/noir/issues/4193 ## Summary\* Uses the new TypeVariableKind::Integer in for loops and bitwise operations to prevent `Field` types from being used there. Removes the old `delayed_type_checks` hack. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: kevaundray Co-authored-by: TomAFrench --- .../src/ssa/function_builder/mod.rs | 5 + .../src/ssa/ir/instruction/call.rs | 12 +- compiler/noirc_evaluator/src/ssa/ir/types.rs | 5 + .../src/ssa/ssa_gen/context.rs | 10 +- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 9 +- compiler/noirc_frontend/src/ast/expression.rs | 18 +-- .../src/hir/resolution/resolver.rs | 2 +- .../noirc_frontend/src/hir/type_check/expr.rs | 148 +++++++----------- .../noirc_frontend/src/hir/type_check/mod.rs | 31 +--- .../noirc_frontend/src/hir/type_check/stmt.rs | 13 +- compiler/noirc_frontend/src/hir_def/expr.rs | 13 -- compiler/noirc_frontend/src/hir_def/types.rs | 17 +- .../src/monomorphization/mod.rs | 7 +- compiler/noirc_frontend/src/tests.rs | 2 +- noir_stdlib/src/array.nr | 10 +- noir_stdlib/src/collections/bounded_vec.nr | 20 ++- noir_stdlib/src/collections/vec.nr | 8 +- noir_stdlib/src/field.nr | 2 +- noir_stdlib/src/hash/poseidon.nr | 8 +- noir_stdlib/src/hash/poseidon/bn254.nr | 16 +- noir_stdlib/src/slice.nr | 4 +- .../execution_success/array_len/src/main.nr | 6 +- .../brillig_cow_regression/src/main.nr | 14 +- .../brillig_oracle/Prover.toml | 2 +- .../brillig_oracle/src/main.nr | 4 +- .../brillig_slices/src/main.nr | 2 +- .../global_consts/src/baz.nr | 2 +- .../global_consts/src/foo.nr | 6 +- .../global_consts/src/foo/bar.nr | 4 +- .../global_consts/src/main.nr | 14 +- .../slice_dynamic_index/src/main.nr | 6 +- .../execution_success/slices/src/main.nr | 2 +- 32 files changed, 184 insertions(+), 238 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index fe71b876879..9d27554dcaa 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -115,6 +115,11 @@ impl FunctionBuilder { self.numeric_constant(value.into(), Type::field()) } + /// Insert a numeric constant into the current function of type Type::length_type() + pub(crate) fn length_constant(&mut self, value: impl Into) -> ValueId { + self.numeric_constant(value.into(), Type::length_type()) + } + /// Insert an array constant into the current function with the given element values. pub(crate) fn array_constant(&mut self, elements: im::Vector, typ: Type) -> ValueId { self.current_function.dfg.make_array(elements, typ) diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index 4217a3d4710..9349d58c4d9 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -77,7 +77,7 @@ pub(super) fn simplify_call( Intrinsic::ArrayLen => { if let Some(length) = dfg.try_get_array_length(arguments[0]) { let length = FieldElement::from(length as u128); - SimplifyResult::SimplifiedTo(dfg.make_constant(length, Type::field())) + SimplifyResult::SimplifiedTo(dfg.make_constant(length, Type::length_type())) } else if matches!(dfg.type_of_value(arguments[1]), Type::Slice(_)) { SimplifyResult::SimplifiedTo(arguments[0]) } else { @@ -283,7 +283,7 @@ fn update_slice_length( operator: BinaryOp, block: BasicBlockId, ) -> ValueId { - let one = dfg.make_constant(FieldElement::one(), Type::field()); + let one = dfg.make_constant(FieldElement::one(), Type::length_type()); let instruction = Instruction::Binary(Binary { lhs: slice_len, operator, rhs: one }); let call_stack = dfg.get_value_call_stack(slice_len); dfg.insert_instruction_and_results(instruction, block, None, call_stack).first() @@ -296,8 +296,8 @@ fn simplify_slice_push_back( dfg: &mut DataFlowGraph, block: BasicBlockId, ) -> SimplifyResult { - // The capacity must be an integer so that we can compare it against the slice length which is represented as a field - let capacity = dfg.make_constant((slice.len() as u128).into(), Type::unsigned(64)); + // The capacity must be an integer so that we can compare it against the slice length + let capacity = dfg.make_constant((slice.len() as u128).into(), Type::length_type()); let len_equals_capacity_instr = Instruction::Binary(Binary { lhs: arguments[0], operator: BinaryOp::Eq, rhs: capacity }); let call_stack = dfg.get_value_call_stack(arguments[0]); @@ -362,7 +362,7 @@ fn simplify_slice_pop_back( let new_slice_length = update_slice_length(arguments[0], dfg, BinaryOp::Sub, block); - let element_size = dfg.make_constant((element_count as u128).into(), Type::field()); + let element_size = dfg.make_constant((element_count as u128).into(), Type::length_type()); let flattened_len_instr = Instruction::binary(BinaryOp::Mul, arguments[0], element_size); let mut flattened_len = dfg .insert_instruction_and_results(flattened_len_instr, block, None, CallStack::new()) @@ -478,7 +478,7 @@ fn make_constant_slice( let typ = Type::Slice(Rc::new(vec![typ])); let length = FieldElement::from(result_constants.len() as u128); - (dfg.make_constant(length, Type::field()), dfg.make_array(result_constants.into(), typ)) + (dfg.make_constant(length, Type::length_type()), dfg.make_array(result_constants.into(), typ)) } /// Returns a slice (represented by a tuple (len, slice)) of constants corresponding to the limbs of the radix decomposition. diff --git a/compiler/noirc_evaluator/src/ssa/ir/types.rs b/compiler/noirc_evaluator/src/ssa/ir/types.rs index 8dc9e67db79..ea3f5393245 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/types.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/types.rs @@ -90,6 +90,11 @@ impl Type { Type::Numeric(NumericType::NativeField) } + /// Creates the type of an array's length. + pub(crate) fn length_type() -> Type { + Type::unsigned(64) + } + /// Returns the bit size of the provided numeric type. /// /// # Panics diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index 845ffd15413..9c760c013a9 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -192,7 +192,7 @@ impl<'a> FunctionContext<'a> { ast::Type::Slice(elements) => { let element_types = Self::convert_type(elements).flatten(); Tree::Branch(vec![ - Tree::Leaf(f(Type::field())), + Tree::Leaf(f(Type::length_type())), Tree::Leaf(f(Type::Slice(Rc::new(element_types)))), ]) } @@ -640,13 +640,13 @@ impl<'a> FunctionContext<'a> { let result_alloc = self.builder.set_location(location).insert_allocate(Type::bool()); let true_value = self.builder.numeric_constant(1u128, Type::bool()); self.builder.insert_store(result_alloc, true_value); - let zero = self.builder.field_constant(0u128); + let zero = self.builder.length_constant(0u128); self.builder.terminate_with_jmp(loop_start, vec![zero]); // loop_start self.builder.switch_to_block(loop_start); - let i = self.builder.add_block_parameter(loop_start, Type::field()); - let array_length = self.builder.field_constant(array_length as u128); + let i = self.builder.add_block_parameter(loop_start, Type::length_type()); + let array_length = self.builder.length_constant(array_length as u128); let v0 = self.builder.insert_binary(i, BinaryOp::Lt, array_length); self.builder.terminate_with_jmpif(v0, loop_body, loop_end); @@ -658,7 +658,7 @@ impl<'a> FunctionContext<'a> { let v4 = self.builder.insert_load(result_alloc, Type::bool()); let v5 = self.builder.insert_binary(v4, BinaryOp::And, v3); self.builder.insert_store(result_alloc, v5); - let one = self.builder.field_constant(1u128); + let one = self.builder.length_constant(1u128); let v6 = self.builder.insert_binary(i, BinaryOp::Add, one); self.builder.terminate_with_jmp(loop_start, vec![v6]); diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 8f2c923d62c..6f59fa13274 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -128,6 +128,7 @@ impl<'a> FunctionContext<'a> { } fn codegen_expression(&mut self, expr: &Expression) -> Result { + eprintln!("Codegen {expr}"); match expr { Expression::Ident(ident) => Ok(self.codegen_ident(ident)), Expression::Literal(literal) => self.codegen_literal(literal), @@ -196,7 +197,7 @@ impl<'a> FunctionContext<'a> { } ast::Type::Slice(_) => { let slice_length = - self.builder.field_constant(array.contents.len() as u128); + self.builder.length_constant(array.contents.len() as u128); let slice_contents = self.codegen_array_checked(elements, typ[1].clone())?; Tree::Branch(vec![slice_length.into(), slice_contents]) @@ -221,7 +222,7 @@ impl<'a> FunctionContext<'a> { // A caller needs multiple pieces of information to make use of a format string // The message string, the number of fields to be formatted, and the fields themselves let string = self.codegen_string(string); - let field_count = self.builder.field_constant(*number_of_fields as u128); + let field_count = self.builder.length_constant(*number_of_fields as u128); let fields = self.codegen_expression(fields)?; Ok(Tree::Branch(vec![string, field_count.into(), fields])) @@ -347,8 +348,10 @@ impl<'a> FunctionContext<'a> { } fn codegen_binary(&mut self, binary: &ast::Binary) -> Result { + eprintln!("Start binary"); let lhs = self.codegen_non_tuple_expression(&binary.lhs)?; let rhs = self.codegen_non_tuple_expression(&binary.rhs)?; + eprintln!("Insert binary"); Ok(self.insert_binary(lhs, binary.operator, rhs, binary.location)) } @@ -615,7 +618,7 @@ impl<'a> FunctionContext<'a> { { match intrinsic { Intrinsic::SliceInsert => { - let one = self.builder.field_constant(1u128); + let one = self.builder.length_constant(1u128); // We add one here in the case of a slice insert as a slice insert at the length of the slice // can be converted to a slice push back diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index c78deaf6dbb..2a252633a29 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -236,7 +236,15 @@ impl BinaryOpKind { } pub fn is_valid_for_field_type(self) -> bool { - matches!(self, BinaryOpKind::Equal | BinaryOpKind::NotEqual) + matches!( + self, + BinaryOpKind::Add + | BinaryOpKind::Subtract + | BinaryOpKind::Multiply + | BinaryOpKind::Divide + | BinaryOpKind::Equal + | BinaryOpKind::NotEqual + ) } pub fn as_string(self) -> &'static str { @@ -280,14 +288,6 @@ impl BinaryOpKind { BinaryOpKind::Modulo => Token::Percent, } } - - pub fn is_bit_shift(&self) -> bool { - matches!(self, BinaryOpKind::ShiftRight | BinaryOpKind::ShiftLeft) - } - - pub fn is_modulo(&self) -> bool { - matches!(self, BinaryOpKind::Modulo) - } } #[derive(PartialEq, PartialOrd, Eq, Ord, Hash, Debug, Copy, Clone)] diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index f05a69be7c2..7f9e48353a7 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1464,7 +1464,7 @@ impl<'a> Resolver<'a> { // checker does not check definition kinds and otherwise expects // parameters to already be typed. if self.interner.definition_type(hir_ident.id) == Type::Error { - let typ = Type::polymorphic_integer(self.interner); + let typ = Type::polymorphic_integer_or_field(self.interner); self.interner.push_definition_type(hir_ident.id, typ); } } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index a669a4a246e..b78f07c88f2 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -104,7 +104,7 @@ impl<'interner> TypeChecker<'interner> { Type::Array(Box::new(length), Box::new(elem_type)) } HirLiteral::Bool(_) => Type::Bool, - HirLiteral::Integer(_, _) => Type::polymorphic_integer(self.interner), + HirLiteral::Integer(_, _) => Type::polymorphic_integer_or_field(self.interner), HirLiteral::Str(string) => { let len = Type::Constant(string.len() as u64); Type::String(Box::new(len)) @@ -529,13 +529,15 @@ impl<'interner> TypeChecker<'interner> { let index_type = self.check_expression(&index_expr.index); let span = self.interner.expr_span(&index_expr.index); - index_type.unify(&Type::polymorphic_integer(self.interner), &mut self.errors, || { - TypeCheckError::TypeMismatch { + index_type.unify( + &Type::polymorphic_integer_or_field(self.interner), + &mut self.errors, + || TypeCheckError::TypeMismatch { expected_typ: "an integer".to_owned(), expr_typ: index_type.to_string(), expr_span: span, - } - }); + }, + ); // When writing `a[i]`, if `a : &mut ...` then automatically dereference `a` as many // times as needed to get the underlying array. @@ -807,43 +809,13 @@ impl<'interner> TypeChecker<'interner> { // Matches on TypeVariable must be first to follow any type // bindings. - (TypeVariable(int, int_kind), other) | (other, TypeVariable(int, int_kind)) => { - if let TypeBinding::Bound(binding) = &*int.borrow() { + (TypeVariable(var, _), other) | (other, TypeVariable(var, _)) => { + if let TypeBinding::Bound(binding) = &*var.borrow() { return self.comparator_operand_type_rules(other, binding, op, span); } - if !op.kind.is_valid_for_field_type() && (other.is_bindable() || other.is_field()) { - let other = other.follow_bindings(); - - self.push_delayed_type_check(Box::new(move || { - if other.is_field() || other.is_bindable() { - Err(TypeCheckError::InvalidComparisonOnField { span }) - } else { - Ok(()) - } - })); - } - - let mut bindings = TypeBindings::new(); - if other - .try_bind_to_polymorphic_int( - int, - &mut bindings, - *int_kind == TypeVariableKind::Integer, - ) - .is_ok() - || other == &Type::Error - { - Type::apply_type_bindings(bindings); - Ok((Bool, false)) - } else { - Err(TypeCheckError::TypeMismatchWithSource { - expected: lhs_type.clone(), - actual: rhs_type.clone(), - span, - source: Source::Binary, - }) - } + self.bind_type_variables_for_infix(lhs_type, op, rhs_type, span); + Ok((Bool, false)) } (Alias(alias, args), other) | (other, Alias(alias, args)) => { let alias = alias.borrow().get_type(args); @@ -1071,6 +1043,38 @@ impl<'interner> TypeChecker<'interner> { } } + fn bind_type_variables_for_infix( + &mut self, + lhs_type: &Type, + op: &HirBinaryOp, + rhs_type: &Type, + span: Span, + ) { + self.unify(lhs_type, rhs_type, || TypeCheckError::TypeMismatchWithSource { + expected: lhs_type.clone(), + actual: rhs_type.clone(), + source: Source::Binary, + span, + }); + + // In addition to unifying both types, we also have to bind either + // the lhs or rhs to an integer type variable. This ensures if both lhs + // and rhs are type variables, that they will have the correct integer + // type variable kind instead of TypeVariableKind::Normal. + let target = if op.kind.is_valid_for_field_type() { + Type::polymorphic_integer_or_field(self.interner) + } else { + Type::polymorphic_integer(self.interner) + }; + + self.unify(lhs_type, &target, || TypeCheckError::TypeMismatchWithSource { + expected: lhs_type.clone(), + actual: rhs_type.clone(), + source: Source::Binary, + span, + }); + } + // Given a binary operator and another type. This method will produce the output type // and a boolean indicating whether to use the trait impl corresponding to the operator // or not. A value of false indicates the caller to use a primitive operation for this @@ -1093,58 +1097,15 @@ impl<'interner> TypeChecker<'interner> { // Matches on TypeVariable must be first so that we follow any type // bindings. - (TypeVariable(int, int_kind), other) | (other, TypeVariable(int, int_kind)) => { + (TypeVariable(int, _), other) | (other, TypeVariable(int, _)) => { if let TypeBinding::Bound(binding) = &*int.borrow() { return self.infix_operand_type_rules(binding, op, other, span); } - if (op.is_modulo() || op.is_bitwise()) && (other.is_bindable() || other.is_field()) - { - let other = other.follow_bindings(); - let kind = op.kind; - // This will be an error if these types later resolve to a Field, or stay - // polymorphic as the bit size will be unknown. Delay this error until the function - // finishes resolving so we can still allow cases like `let x: u8 = 1 << 2;`. - self.push_delayed_type_check(Box::new(move || { - if other.is_field() { - if kind == BinaryOpKind::Modulo { - Err(TypeCheckError::FieldModulo { span }) - } else { - Err(TypeCheckError::InvalidBitwiseOperationOnField { span }) - } - } else if other.is_bindable() { - Err(TypeCheckError::AmbiguousBitWidth { span }) - } else if kind.is_bit_shift() && other.is_signed() { - Err(TypeCheckError::TypeCannotBeUsed { - typ: other, - place: "bit shift", - span, - }) - } else { - Ok(()) - } - })); - } - let mut bindings = TypeBindings::new(); - if other - .try_bind_to_polymorphic_int( - int, - &mut bindings, - *int_kind == TypeVariableKind::Integer, - ) - .is_ok() - || other == &Type::Error - { - Type::apply_type_bindings(bindings); - Ok((other.clone(), false)) - } else { - Err(TypeCheckError::TypeMismatchWithSource { - expected: lhs_type.clone(), - actual: rhs_type.clone(), - source: Source::Binary, - span, - }) - } + self.bind_type_variables_for_infix(lhs_type, op, rhs_type, span); + + // Both types are unified so the choice of which to return is arbitrary + Ok((other.clone(), false)) } (Alias(alias, args), other) | (other, Alias(alias, args)) => { let alias = alias.borrow().get_type(args); @@ -1169,11 +1130,12 @@ impl<'interner> TypeChecker<'interner> { } // The result of two Fields is always a witness (FieldElement, FieldElement) => { - if op.is_bitwise() { - return Err(TypeCheckError::InvalidBitwiseOperationOnField { span }); - } - if op.is_modulo() { - return Err(TypeCheckError::FieldModulo { span }); + if !op.kind.is_valid_for_field_type() { + if op.kind == BinaryOpKind::Modulo { + return Err(TypeCheckError::FieldModulo { span }); + } else { + return Err(TypeCheckError::InvalidBitwiseOperationOnField { span }); + } } Ok((FieldElement, false)) } @@ -1213,7 +1175,7 @@ impl<'interner> TypeChecker<'interner> { self.errors .push(TypeCheckError::InvalidUnaryOp { kind: rhs_type.to_string(), span }); } - let expected = Type::polymorphic_integer(self.interner); + let expected = Type::polymorphic_integer_or_field(self.interner); rhs_type.unify(&expected, &mut self.errors, || TypeCheckError::InvalidUnaryOp { kind: rhs_type.to_string(), span, diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 225f5756d7a..21d1c75a0f2 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -21,10 +21,7 @@ use crate::{ use self::errors::Source; -type TypeCheckFn = Box Result<(), TypeCheckError>>; - pub struct TypeChecker<'interner> { - delayed_type_checks: Vec, interner: &'interner mut NodeInterner, errors: Vec, current_function: Option, @@ -80,15 +77,7 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec (noirc_e impl<'interner> TypeChecker<'interner> { fn new(interner: &'interner mut NodeInterner) -> Self { - Self { - delayed_type_checks: Vec::new(), - interner, - errors: Vec::new(), - trait_constraints: Vec::new(), - current_function: None, - } - } - - pub fn push_delayed_type_check(&mut self, f: TypeCheckFn) { - self.delayed_type_checks.push(f); + Self { interner, errors: Vec::new(), trait_constraints: Vec::new(), current_function: None } } - fn check_function_body(&mut self, body: &ExprId) -> (Type, Vec) { - let body_type = self.check_expression(body); - (body_type, std::mem::take(&mut self.delayed_type_checks)) + fn check_function_body(&mut self, body: &ExprId) -> Type { + self.check_expression(body) } pub fn check_global( @@ -198,7 +176,6 @@ impl<'interner> TypeChecker<'interner> { interner: &'interner mut NodeInterner, ) -> Vec { let mut this = Self { - delayed_type_checks: Vec::new(), interner, errors: Vec::new(), trait_constraints: Vec::new(), diff --git a/compiler/noirc_frontend/src/hir/type_check/stmt.rs b/compiler/noirc_frontend/src/hir/type_check/stmt.rs index 370b4ee7b17..358bea86922 100644 --- a/compiler/noirc_frontend/src/hir/type_check/stmt.rs +++ b/compiler/noirc_frontend/src/hir/type_check/stmt.rs @@ -73,13 +73,10 @@ impl<'interner> TypeChecker<'interner> { let expected_type = Type::polymorphic_integer(self.interner); - self.unify(&start_range_type, &expected_type, || { - TypeCheckError::TypeCannotBeUsed { - typ: start_range_type.clone(), - place: "for loop", - span: range_span, - } - .add_context("The range of a loop must be known at compile-time") + self.unify(&start_range_type, &expected_type, || TypeCheckError::TypeCannotBeUsed { + typ: start_range_type.clone(), + place: "for loop", + span: range_span, }); self.interner.push_definition_type(for_loop.identifier.id, start_range_type); @@ -235,7 +232,7 @@ impl<'interner> TypeChecker<'interner> { let expr_span = self.interner.expr_span(index); index_type.unify( - &Type::polymorphic_integer(self.interner), + &Type::polymorphic_integer_or_field(self.interner), &mut self.errors, || TypeCheckError::TypeMismatch { expected_typ: "an integer".to_owned(), diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index 75ed68af0f6..b4c590de491 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -94,19 +94,6 @@ impl HirBinaryOp { let location = Location::new(op.span(), file); HirBinaryOp { location, kind } } - - pub fn is_bitwise(&self) -> bool { - use BinaryOpKind::*; - matches!(self.kind, And | Or | Xor | ShiftRight | ShiftLeft) - } - - pub fn is_bit_shift(&self) -> bool { - self.kind.is_bit_shift() - } - - pub fn is_modulo(&self) -> bool { - self.kind.is_modulo() - } } #[derive(Debug, Clone)] diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index d4d8a948460..e105da1ccf0 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -571,13 +571,20 @@ impl Type { Type::TypeVariable(var, kind) } - pub fn polymorphic_integer(interner: &mut NodeInterner) -> Type { + pub fn polymorphic_integer_or_field(interner: &mut NodeInterner) -> Type { let id = interner.next_type_variable_id(); let kind = TypeVariableKind::IntegerOrField; let var = TypeVariable::unbound(id); Type::TypeVariable(var, kind) } + pub fn polymorphic_integer(interner: &mut NodeInterner) -> Type { + let id = interner.next_type_variable_id(); + let kind = TypeVariableKind::Integer; + let var = TypeVariable::unbound(id); + Type::TypeVariable(var, kind) + } + /// A bit of an awkward name for this function - this function returns /// true for type variables or polymorphic integers which are unbound. /// NamedGenerics will always be false as although they are bindable, @@ -964,7 +971,7 @@ impl Type { /// Try to bind a PolymorphicInt variable to self, succeeding if self is an integer, field, /// other PolymorphicInt type, or type variable. If successful, the binding is placed in the /// given TypeBindings map rather than linked immediately. - pub fn try_bind_to_polymorphic_int( + fn try_bind_to_polymorphic_int( &self, var: &TypeVariable, bindings: &mut TypeBindings, @@ -977,7 +984,11 @@ impl Type { let this = self.substitute(bindings).follow_bindings(); match &this { - Type::FieldElement | Type::Integer(..) => { + Type::Integer(..) => { + bindings.insert(target_id, (var.clone(), this)); + Ok(()) + } + Type::FieldElement if !only_integer => { bindings.insert(target_id, (var.clone(), this)); Ok(()) } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 0f243e47bbe..2e714da21c6 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -27,8 +27,8 @@ use crate::{ }, node_interner::{self, DefinitionKind, NodeInterner, StmtId, TraitImplKind, TraitMethodId}, token::FunctionAttribute, - ContractFunctionType, FunctionKind, IntegerBitSize, Type, TypeBinding, TypeBindings, - TypeVariable, TypeVariableKind, UnaryOp, Visibility, + ContractFunctionType, FunctionKind, IntegerBitSize, Signedness, Type, TypeBinding, + TypeBindings, TypeVariable, TypeVariableKind, UnaryOp, Visibility, }; use self::ast::{Definition, FuncId, Function, LocalId, Program}; @@ -1107,7 +1107,8 @@ impl<'interner> Monomorphizer<'interner> { return match opcode.as_str() { "modulus_num_bits" => { let bits = (FieldElement::max_num_bits() as u128).into(); - let typ = ast::Type::Field; + let typ = + ast::Type::Integer(Signedness::Unsigned, IntegerBitSize::SixtyFour); Some(ast::Expression::Literal(ast::Literal::Integer(bits, typ, location))) } "zeroed" => { diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 8a56b337398..c18379f1c26 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -956,7 +956,7 @@ mod test { #[test] fn resolve_for_expr() { let src = r#" - fn main(x : Field) { + fn main(x : u64) { for i in 1..20 { let _z = x + i; }; diff --git a/noir_stdlib/src/array.nr b/noir_stdlib/src/array.nr index 995af6c4c6f..7871b1a6f9a 100644 --- a/noir_stdlib/src/array.nr +++ b/noir_stdlib/src/array.nr @@ -4,10 +4,10 @@ use crate::cmp::{Ord}; // by the methods in the `slice` module impl [T; N] { #[builtin(array_len)] - pub fn len(self) -> Field {} + pub fn len(self) -> u64 {} pub fn sort(self) -> Self where T: Ord { - self.sort_via(|a, b| a <= b) + self.sort_via(|a: T, b: T| a <= b) } pub fn sort_via(self, ordering: fn[Env](T, T) -> bool) -> Self { @@ -31,7 +31,7 @@ impl [T; N] { } /// Returns the index of the elements in the array that would sort it, using the provided custom sorting function. - unconstrained fn get_sorting_index(self, ordering: fn[Env](T, T) -> bool) -> [Field; N] { + unconstrained fn get_sorting_index(self, ordering: fn[Env](T, T) -> bool) -> [u64; N] { let mut result = [0;N]; let mut a = self; for i in 0..N { @@ -117,7 +117,7 @@ impl [T; N] { // helper function used to look up the position of a value in an array of Field // Note that function returns 0 if the value is not found -unconstrained fn find_index(a: [Field;N], find: Field) -> Field { +unconstrained fn find_index(a: [u64; N], find: u64) -> u64 { let mut result = 0; for i in 0..a.len() { if a[i] == find { @@ -125,4 +125,4 @@ unconstrained fn find_index(a: [Field;N], find: Field) -> Field { } } result -} \ No newline at end of file +} diff --git a/noir_stdlib/src/collections/bounded_vec.nr b/noir_stdlib/src/collections/bounded_vec.nr index 332fefa63f9..a4aa4823f38 100644 --- a/noir_stdlib/src/collections/bounded_vec.nr +++ b/noir_stdlib/src/collections/bounded_vec.nr @@ -1,8 +1,6 @@ struct BoundedVec { storage: [T; MaxLen], - // TODO: change this to return a u64 as Noir now - // uses u64 for indexing - len: Field, + len: u64, empty_value: T, } @@ -11,27 +9,27 @@ impl BoundedVec { BoundedVec { storage: [initial_value; MaxLen], len: 0, empty_value: initial_value } } - pub fn get(mut self: Self, index: Field) -> T { - assert(index as u64 < self.len as u64); + pub fn get(mut self: Self, index: u64) -> T { + assert(index as u64 < self.len); self.storage[index] } - pub fn get_unchecked(mut self: Self, index: Field) -> T { + pub fn get_unchecked(mut self: Self, index: u64) -> T { self.storage[index] } pub fn push(&mut self, elem: T) { - assert(self.len as u64 < MaxLen as u64, "push out of bounds"); + assert(self.len < MaxLen as u64, "push out of bounds"); self.storage[self.len] = elem; self.len += 1; } - pub fn len(self) -> Field { + pub fn len(self) -> u64 { self.len } - pub fn max_len(_self: BoundedVec) -> Field { + pub fn max_len(_self: BoundedVec) -> u64{ MaxLen } @@ -59,7 +57,7 @@ impl BoundedVec { for i in 0..Len { exceeded_len |= i == append_len; if !exceeded_len { - self.storage[self.len + (i as Field)] = vec.get_unchecked(i as Field); + self.storage[self.len + i] = vec.get_unchecked(i); } } self.len = new_len; @@ -85,4 +83,4 @@ impl BoundedVec { } ret } -} \ No newline at end of file +} diff --git a/noir_stdlib/src/collections/vec.nr b/noir_stdlib/src/collections/vec.nr index 43d68e1d1e7..2e7945be827 100644 --- a/noir_stdlib/src/collections/vec.nr +++ b/noir_stdlib/src/collections/vec.nr @@ -17,7 +17,7 @@ impl Vec { /// Get an element from the vector at the given index. /// Panics if the given index /// points beyond the end of the vector. - pub fn get(self, index: Field) -> T { + pub fn get(self, index: u64) -> T { self.slice[index] } @@ -40,20 +40,20 @@ impl Vec { /// Insert an element at a specified index, shifting all elements /// after it to the right - pub fn insert(&mut self, index: Field, elem: T) { + pub fn insert(&mut self, index: u64, elem: T) { self.slice = self.slice.insert(index, elem); } /// Remove an element at a specified index, shifting all elements /// after it to the left, returning the removed element - pub fn remove(&mut self, index: Field) -> T { + pub fn remove(&mut self, index: u64) -> T { let (new_slice, elem) = self.slice.remove(index); self.slice = new_slice; elem } /// Returns the number of elements in the vector - pub fn len(self) -> Field { + pub fn len(self) -> u64 { self.slice.len() } } diff --git a/noir_stdlib/src/field.nr b/noir_stdlib/src/field.nr index 66fb50119f9..a7278d85999 100644 --- a/noir_stdlib/src/field.nr +++ b/noir_stdlib/src/field.nr @@ -89,7 +89,7 @@ impl Field { } #[builtin(modulus_num_bits)] -pub fn modulus_num_bits() -> Field {} +pub fn modulus_num_bits() -> u64 {} #[builtin(modulus_be_bits)] pub fn modulus_be_bits() -> [u1] {} diff --git a/noir_stdlib/src/hash/poseidon.nr b/noir_stdlib/src/hash/poseidon.nr index 3f4de73c0db..b1a7c4a2367 100644 --- a/noir_stdlib/src/hash/poseidon.nr +++ b/noir_stdlib/src/hash/poseidon.nr @@ -21,7 +21,7 @@ pub fn config( // Input checks let mul = crate::wrapping_mul(t as u8, (rf + rp)); assert(mul == ark.len() as u8); - assert(t * t == mds.len()); + assert(t * t == mds.len() as Field); assert(alpha != 0); PoseidonConfig { t, rf, rp, alpha, ark, mds } @@ -30,7 +30,7 @@ pub fn config( fn permute(pos_conf: PoseidonConfig, mut state: [Field; O]) -> [Field; O] { let PoseidonConfig {t, rf, rp, alpha, ark, mds} = pos_conf; - assert(t == state.len()); + assert(t == state.len() as Field); let mut count = 0; // for r in 0..rf + rp @@ -47,7 +47,7 @@ fn permute(pos_conf: PoseidonConfig, mut state: [Field; O]) -> [F } state = apply_matrix(mds, state); // Apply MDS matrix - count = count + t; + count = count + t as u64; } state @@ -85,7 +85,7 @@ fn absorb( fn check_security(rate: Field, width: Field, security: Field) -> bool { let n = modulus_num_bits(); - ((n - 1) * (width - rate) / 2) as u8 > security as u8 + ((n - 1) as Field * (width - rate) / 2) as u8 > security as u8 } // A*x where A is an n x n matrix in row-major order and x an n-vector fn apply_matrix(a: [Field; M], x: [Field; N]) -> [Field; N] { diff --git a/noir_stdlib/src/hash/poseidon/bn254.nr b/noir_stdlib/src/hash/poseidon/bn254.nr index 0db6d9546dc..37b08e3c8fb 100644 --- a/noir_stdlib/src/hash/poseidon/bn254.nr +++ b/noir_stdlib/src/hash/poseidon/bn254.nr @@ -9,12 +9,12 @@ use crate::hash::poseidon::apply_matrix; #[field(bn254)] pub fn permute(pos_conf: PoseidonConfig, mut state: [Field; O]) -> [Field; O] { let PoseidonConfig {t, rf: config_rf, rp: config_rp, alpha, ark, mds} = pos_conf; - let rf = 8; - let rp = [56, 57, 56, 60, 60, 63, 64, 63, 60, 66, 60, 65, 70, 60, 64, 68][state.len() - 2]; + let rf: u8 = 8; + let rp: u8 = [56, 57, 56, 60, 60, 63, 64, 63, 60, 66, 60, 65, 70, 60, 64, 68][state.len() - 2]; - assert(t == state.len()); - assert(rf == config_rf as Field); - assert(rp == config_rp as Field); + assert(t == state.len() as Field); + assert(rf == config_rf); + assert(rp == config_rp); let mut count = 0; // First half of full rounds @@ -27,7 +27,7 @@ pub fn permute(pos_conf: PoseidonConfig, mut state: [Field; O]) - } state = apply_matrix(mds, state); // Apply MDS matrix - count = count + t; + count = count + t as u64; } // Partial rounds for _r in 0..rp { @@ -37,7 +37,7 @@ pub fn permute(pos_conf: PoseidonConfig, mut state: [Field; O]) - state[0] = state[0].pow_32(alpha); state = apply_matrix(mds, state); // Apply MDS matrix - count = count + t; + count = count + t as u64; } // Second half of full rounds for _r in 0..rf / 2 { @@ -49,7 +49,7 @@ pub fn permute(pos_conf: PoseidonConfig, mut state: [Field; O]) - } state = apply_matrix(mds, state); // Apply MDS matrix - count = count + t; + count = count + t as u64; } state diff --git a/noir_stdlib/src/slice.nr b/noir_stdlib/src/slice.nr index aa4b73edc1a..bb5c43e497b 100644 --- a/noir_stdlib/src/slice.nr +++ b/noir_stdlib/src/slice.nr @@ -24,13 +24,13 @@ impl [T] { /// Insert an element at a specified index, shifting all elements /// after it to the right #[builtin(slice_insert)] - pub fn insert(self, index: Field, elem: T) -> Self { } + pub fn insert(self, index: u64, elem: T) -> Self { } /// Remove an element at a specified index, shifting all elements /// after it to the left, returning the altered slice and /// the removed element #[builtin(slice_remove)] - pub fn remove(self, index: Field) -> (Self, T) { } + pub fn remove(self, index: u64) -> (Self, T) { } // Append each element of the `other` slice to the end of `self`. // This returns a new slice and leaves both input slices unchanged. diff --git a/test_programs/execution_success/array_len/src/main.nr b/test_programs/execution_success/array_len/src/main.nr index b60762f4636..f846cfb9844 100644 --- a/test_programs/execution_success/array_len/src/main.nr +++ b/test_programs/execution_success/array_len/src/main.nr @@ -1,12 +1,12 @@ -fn len_plus_1(array: [T; N]) -> Field { +fn len_plus_1(array: [T; N]) -> u64 { array.len() + 1 } -fn add_lens(a: [T; N], b: [Field; M]) -> Field { +fn add_lens(a: [T; N], b: [Field; M]) -> u64 { a.len() + b.len() } -fn nested_call(b: [Field; N]) -> Field { +fn nested_call(b: [Field; N]) -> u64 { len_plus_1(b) } diff --git a/test_programs/execution_success/brillig_cow_regression/src/main.nr b/test_programs/execution_success/brillig_cow_regression/src/main.nr index 974c17dfbc9..74aeda18261 100644 --- a/test_programs/execution_success/brillig_cow_regression/src/main.nr +++ b/test_programs/execution_success/brillig_cow_regression/src/main.nr @@ -1,12 +1,12 @@ // Tests a performance regression found in aztec-packages with brillig cow optimization -global MAX_NEW_COMMITMENTS_PER_TX: Field = 64; -global MAX_NEW_NULLIFIERS_PER_TX: Field = 64; -global MAX_NEW_L2_TO_L1_MSGS_PER_TX: Field = 2; -global MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX: Field = 16; -global MAX_NEW_CONTRACTS_PER_TX: Field = 1; -global NUM_ENCRYPTED_LOGS_HASHES_PER_TX: Field = 1; -global NUM_UNENCRYPTED_LOGS_HASHES_PER_TX: Field = 1; +global MAX_NEW_COMMITMENTS_PER_TX = 64; +global MAX_NEW_NULLIFIERS_PER_TX = 64; +global MAX_NEW_L2_TO_L1_MSGS_PER_TX = 2; +global MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX = 16; +global MAX_NEW_CONTRACTS_PER_TX = 1; +global NUM_ENCRYPTED_LOGS_HASHES_PER_TX = 1; +global NUM_UNENCRYPTED_LOGS_HASHES_PER_TX = 1; global NUM_FIELDS_PER_SHA256 = 2; global CALLDATA_HASH_INPUT_SIZE = 169; global CALL_DATA_HASH_LOG_FIELDS = 4; diff --git a/test_programs/execution_success/brillig_oracle/Prover.toml b/test_programs/execution_success/brillig_oracle/Prover.toml index 2b26a4ce471..161f4fb62c0 100644 --- a/test_programs/execution_success/brillig_oracle/Prover.toml +++ b/test_programs/execution_success/brillig_oracle/Prover.toml @@ -1,2 +1,2 @@ -x = "10" +_x = "10" diff --git a/test_programs/execution_success/brillig_oracle/src/main.nr b/test_programs/execution_success/brillig_oracle/src/main.nr index 490b7b605e3..6a9e5806621 100644 --- a/test_programs/execution_success/brillig_oracle/src/main.nr +++ b/test_programs/execution_success/brillig_oracle/src/main.nr @@ -2,7 +2,7 @@ use dep::std::slice; use dep::std::test::OracleMock; // Tests oracle usage in brillig/unconstrained functions -fn main(x: Field) { +fn main(_x: Field) { let size = 20; // TODO: Add a method along the lines of `(0..size).to_array()`. let mut mock_oracle_response = [0; 20]; @@ -17,7 +17,7 @@ fn main(x: Field) { let _ = OracleMock::mock("get_number_sequence").with_params(size).returns((20, mock_oracle_response)); let _ = OracleMock::mock("get_reverse_number_sequence").with_params(size).returns((20, reversed_mock_oracle_response)); - get_number_sequence_wrapper(size); + get_number_sequence_wrapper(size as Field); } // Define oracle functions which we have mocked above diff --git a/test_programs/execution_success/brillig_slices/src/main.nr b/test_programs/execution_success/brillig_slices/src/main.nr index 48bc8a76bb8..847c41de25c 100644 --- a/test_programs/execution_success/brillig_slices/src/main.nr +++ b/test_programs/execution_success/brillig_slices/src/main.nr @@ -131,7 +131,7 @@ unconstrained fn merge_slices_mutate_in_loop(x: Field, y: Field) -> [Field] { let mut slice = [0; 2]; if x != y { for i in 0..5 { - slice = slice.push_back(i); + slice = slice.push_back(i as Field); } } else { slice = slice.push_back(x); diff --git a/test_programs/execution_success/global_consts/src/baz.nr b/test_programs/execution_success/global_consts/src/baz.nr index 4271de81118..384cf9d3569 100644 --- a/test_programs/execution_success/global_consts/src/baz.nr +++ b/test_programs/execution_success/global_consts/src/baz.nr @@ -1,5 +1,5 @@ pub fn from_baz(x: [Field; crate::foo::MAGIC_NUMBER]) { for i in 0..crate::foo::MAGIC_NUMBER { - assert(x[i] == crate::foo::MAGIC_NUMBER); + assert(x[i] == crate::foo::MAGIC_NUMBER as Field); } } diff --git a/test_programs/execution_success/global_consts/src/foo.nr b/test_programs/execution_success/global_consts/src/foo.nr index 7b0ae75b74b..413b9c3a74b 100644 --- a/test_programs/execution_success/global_consts/src/foo.nr +++ b/test_programs/execution_success/global_consts/src/foo.nr @@ -1,11 +1,11 @@ mod bar; -global N: Field = 5; -global MAGIC_NUMBER: Field = 3; +global N: u64 = 5; +global MAGIC_NUMBER: u64 = 3; global TYPE_INFERRED = 42; pub fn from_foo(x: [Field; bar::N]) { for i in 0..bar::N { - assert(x[i] == bar::N); + assert(x[i] == bar::N as Field); } } diff --git a/test_programs/execution_success/global_consts/src/foo/bar.nr b/test_programs/execution_success/global_consts/src/foo/bar.nr index b8d0b85b0f3..5404c9cf1e3 100644 --- a/test_programs/execution_success/global_consts/src/foo/bar.nr +++ b/test_programs/execution_success/global_consts/src/foo/bar.nr @@ -1,5 +1,5 @@ -global N: Field = 5; +global N: u64 = 5; pub fn from_bar(x: Field) -> Field { - x * N + x * N as Field } diff --git a/test_programs/execution_success/global_consts/src/main.nr b/test_programs/execution_success/global_consts/src/main.nr index 25cc0e4dd36..3c8ecc67a0c 100644 --- a/test_programs/execution_success/global_consts/src/main.nr +++ b/test_programs/execution_success/global_consts/src/main.nr @@ -3,7 +3,7 @@ mod baz; global M: Field = 32; global L: Field = 10; // Unused globals currently allowed -global N: Field = 5; +global N: u64 = 5; global T_LEN = 2; // Type inference is allowed on globals // Globals can reference other globals @@ -36,12 +36,12 @@ fn main( let test_struct = Dummy { x: d, y: c }; for i in 0..foo::MAGIC_NUMBER { - assert(c[i] == foo::MAGIC_NUMBER); - assert(test_struct.y[i] == foo::MAGIC_NUMBER); + assert(c[i] == foo::MAGIC_NUMBER as Field); + assert(test_struct.y[i] == foo::MAGIC_NUMBER as Field); assert(test_struct.y[i] != NESTED[1][0].v); } - assert(N != M); + assert(N as Field != M); let expected: u32 = 42; assert(foo::TYPE_INFERRED == expected); @@ -62,12 +62,12 @@ fn main( arrays_neq(a, b); - let t: [Field; T_LEN] = [N, M]; + let t: [Field; T_LEN] = [N as Field, M]; assert(t[1] == 32); assert(15 == my_submodule::my_helper()); - let add_submodules_N = my_submodule::N + foo::bar::N; + let add_submodules_N = my_submodule::N + foo::bar::N as Field; assert(15 == add_submodules_N); let add_from_bar_N = my_submodule::N + foo::bar::from_bar(1); assert(15 == add_from_bar_N); @@ -75,7 +75,7 @@ fn main( let sugared = [0; my_submodule::N + 2]; assert(sugared[my_submodule::N + 1] == 0); - let arr: [Field; my_submodule::N] = [N; 10]; + let arr: [Field; my_submodule::N] = [N as Field; 10]; assert((arr[0] == 5) & (arr[9] == 5)); foo::from_foo(d); diff --git a/test_programs/execution_success/slice_dynamic_index/src/main.nr b/test_programs/execution_success/slice_dynamic_index/src/main.nr index 374d2ba4c26..41fc9a645c1 100644 --- a/test_programs/execution_success/slice_dynamic_index/src/main.nr +++ b/test_programs/execution_success/slice_dynamic_index/src/main.nr @@ -6,7 +6,7 @@ fn main(x: Field) { fn regression_dynamic_slice_index(x: Field, y: Field) { let mut slice = []; for i in 0..5 { - slice = slice.push_back(i); + slice = slice.push_back(i as Field); } assert(slice.len() == 5); @@ -124,12 +124,12 @@ fn dynamic_slice_merge_if(mut slice: [Field], x: Field) { assert(first_elem == 12); assert(rest_of_slice.len() == 6); - slice = rest_of_slice.insert(x - 2, 20); + slice = rest_of_slice.insert(x as u64 - 2, 20); assert(slice[2] == 20); assert(slice[6] == 30); assert(slice.len() == 7); - let (removed_slice, removed_elem) = slice.remove(x - 1); + let (removed_slice, removed_elem) = slice.remove(x as u64 - 1); // The deconstructed tuple assigns to the slice but is not seen outside of the if statement // without a direct assignment slice = removed_slice; diff --git a/test_programs/execution_success/slices/src/main.nr b/test_programs/execution_success/slices/src/main.nr index c377d2e5b2f..eca42a660c4 100644 --- a/test_programs/execution_success/slices/src/main.nr +++ b/test_programs/execution_success/slices/src/main.nr @@ -167,7 +167,7 @@ fn merge_slices_mutate_in_loop(x: Field, y: Field) -> [Field] { let mut slice = [0; 2]; if x != y { for i in 0..5 { - slice = slice.push_back(i); + slice = slice.push_back(i as Field); } } else { slice = slice.push_back(x);