feat: Sync from noir (#6086)

Automated pull of development from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE feat: add de-sugaring for `impl Trait` in function parameters (noir-lang/noir#4919) feat(experimental): `comptime` globals (noir-lang/noir#4918) chore: update error conversion traits to act on references (noir-lang/noir#4936) fix: ban self-referential structs (noir-lang/noir#4883) chore: add regression test for #3051 (noir-lang/noir#4815) fix: discard ref counts during unrolling (noir-lang/noir#4923) feat!: Bit shift is restricted to u8 right operand (noir-lang/noir#4907) END_COMMIT_OVERRIDE --------- Co-authored-by: Tom French <[email protected]> Co-authored-by: Tom French <[email protected]>
AztecProtocol · May 1, 2024 · f060fa6 · f060fa6
1 parent 67485f1
commit f060fa6
Show file tree

Hide file tree

Showing 71 changed files with 1,038 additions and 293 deletions.
diff --git a/.noir-sync-commit b/.noir-sync-commit
@@ -1 +1 @@
-1ec9cdc7013e867db3672d27e3a6104e4b7e7eef
+8aad2e45acbe08afc3902db95a83324f822c35eb
diff --git a/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr
@@ -116,12 +116,12 @@ contract AvmTest {
 
     #[aztec(public-vm)]
     fn set_opcode_u32() -> pub u32 {
-        1 << 30 as u32
+        1 << 30 as u8
     }
 
     #[aztec(public-vm)]
     fn set_opcode_u64() -> pub u64 {
-        1 << 60 as u64
+        1 << 60 as u8
     }
 
     #[aztec(public-vm)]

diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/append_only_tree.nr b/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/append_only_tree.nr
@@ -12,7 +12,7 @@ pub fn insert_subtree_to_snapshot_tree<N>(
 ) -> AppendOnlyTreeSnapshot {
     // TODO(Lasse): Sanity check len of sibling_path > height of subtree
     // TODO(Lasse): Ensure height of subtree is correct (eg 3 for commitments, 1 for contracts)
-    let leaf_index_at_depth = snapshot.next_available_leaf_index >> (subtree_depth as u32);
+    let leaf_index_at_depth = snapshot.next_available_leaf_index >> (subtree_depth as u8);
 
     // Check that the current root is correct and that there is an empty subtree at the insertion location
     assert_check_membership(
@@ -30,7 +30,7 @@ pub fn insert_subtree_to_snapshot_tree<N>(
     );
 
     // 2^subtree_depth is the number of leaves added. 2^x = 1 << x
-    let new_next_available_leaf_index = (snapshot.next_available_leaf_index as u64) + (1 << (subtree_depth as u64));
+    let new_next_available_leaf_index = (snapshot.next_available_leaf_index as u64) + (1 << (subtree_depth as u8));
 
     AppendOnlyTreeSnapshot { root: new_root, next_available_leaf_index: new_next_available_leaf_index as u32 }
 }
diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/indexed_tree.nr b/noir-projects/noir-protocol-circuits/crates/types/src/merkle_tree/indexed_tree.nr
@@ -64,7 +64,7 @@ pub fn batch_insert<Value, Leaf, SubtreeWidth, SiblingPathLength, SubtreeHeight,
     }
 
     let empty_subtree_root = calculate_empty_tree_root(SubtreeHeight);
-    let leaf_index_subtree_depth = start_insertion_index >> (SubtreeHeight as u32);
+    let leaf_index_subtree_depth = start_insertion_index >> (SubtreeHeight as u8);
 
     assert_check_membership(
         empty_subtree_root,
@@ -78,7 +78,7 @@ pub fn batch_insert<Value, Leaf, SubtreeWidth, SiblingPathLength, SubtreeHeight,
 
     // Calculate the new root
     // We are inserting a subtree rather than a full tree here
-    let subtree_index = start_insertion_index >> (SubtreeHeight as u32);
+    let subtree_index = start_insertion_index >> (SubtreeHeight as u8);
     let new_root = root_from_sibling_path(subtree_root, subtree_index as Field, new_subtree_sibling_path);
 
     AppendOnlyTreeSnapshot { root: new_root, next_available_leaf_index: start_insertion_index + (values_to_insert.len() as u32) }

diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/tests/merkle_tree_utils.nr b/noir-projects/noir-protocol-circuits/crates/types/src/tests/merkle_tree_utils.nr
@@ -1,7 +1,4 @@
-use crate::{
-    merkle_tree::{MerkleTree, calculate_empty_tree_root},
-    traits::Empty
-};
+use crate::{merkle_tree::{MerkleTree, calculate_empty_tree_root}, traits::Empty};
 
 pub fn compute_zero_hashes<N>(mut hashes: [Field; N]) -> [Field; N] {
     hashes[0] = dep::std::hash::pedersen_hash([0, 0]);
@@ -111,7 +108,7 @@ impl<SUBTREE_ITEMS, TREE_HEIGHT, SUPERTREE_HEIGHT, SUBTREE_HEIGHT> NonEmptyMerkl
         left_supertree_branch[0] = dep::std::hash::pedersen_hash([subtree.get_root(), zero_hashes[SUBTREE_HEIGHT-1]]);
         for i in 1..left_supertree_branch.len() {
             // TODO(md): far right of this yuck
-            left_supertree_branch[i] = dep::std::hash::pedersen_hash([left_supertree_branch[i-1], zero_hashes[(SUBTREE_HEIGHT-1 as u32) + i as u32]]);
+            left_supertree_branch[i] = dep::std::hash::pedersen_hash([left_supertree_branch[i-1], zero_hashes[(SUBTREE_HEIGHT as u64 -1) + i as u64]]);
         }
 
         NonEmptyMerkleTree { subtree, zero_hashes, left_supertree_branch, _phantom_subtree_height: _subtree_height }

diff --git a/noir/noir-repo/.github/workflows/publish-nargo.yml b/noir/noir-repo/.github/workflows/publish-nargo.yml
@@ -23,7 +23,7 @@ permissions:
 
 jobs:
   build-apple-darwin:
-    runs-on: macos-latest
+    runs-on: macos-12
     env:
       CROSS_CONFIG: ${{ github.workspace }}/.github/Cross.toml
       NIGHTLY_RELEASE: ${{ inputs.tag == '' }}

diff --git a/noir/noir-repo/acvm-repo/brillig_vm/src/arithmetic.rs b/noir/noir-repo/acvm-repo/brillig_vm/src/arithmetic.rs
@@ -71,7 +71,9 @@ pub(crate) fn evaluate_binary_int_op(
             }
         }
     })?;
-    let rhs = rhs.expect_integer_with_bit_size(bit_size).map_err(|err| match err {
+    let rhs_bit_size =
+        if op == &BinaryIntOp::Shl || op == &BinaryIntOp::Shr { 8 } else { bit_size };
+    let rhs = rhs.expect_integer_with_bit_size(rhs_bit_size).map_err(|err| match err {
         MemoryTypeError::MismatchedBitSize { value_bit_size, expected_bit_size } => {
             BrilligArithmeticError::MismatchedRhsBitSize {
                 rhs_bit_size: value_bit_size,

diff --git a/noir/noir-repo/aztec_macros/src/utils/hir_utils.rs b/noir/noir-repo/aztec_macros/src/utils/hir_utils.rs
@@ -228,6 +228,7 @@ pub fn inject_global(
         module_id,
         file_id,
         global.attributes.clone(),
+        false,
     );
 
     // Add the statement to the scope so its path can be looked up later

diff --git a/noir/noir-repo/compiler/noirc_arena/src/lib.rs b/noir/noir-repo/compiler/noirc_arena/src/lib.rs
@@ -3,6 +3,8 @@
 #![warn(unreachable_pub)]
 #![warn(clippy::semicolon_if_nothing_returned)]
 
+use std::fmt;
+
 #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, Hash)]
 pub struct Index(usize);
 
@@ -25,6 +27,12 @@ impl Index {
     }
 }
 
+impl fmt::Display for Index {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        self.0.fmt(f)
+    }
+}
+
 #[derive(Clone, Debug)]
 pub struct Arena<T> {
     pub vec: Vec<T>,

diff --git a/noir/noir-repo/compiler/noirc_driver/src/lib.rs b/noir/noir-repo/compiler/noirc_driver/src/lib.rs
@@ -252,7 +252,7 @@ pub fn check_crate(
     let mut errors = vec![];
     let diagnostics = CrateDefMap::collect_defs(crate_id, context, macros);
     errors.extend(diagnostics.into_iter().map(|(error, file_id)| {
-        let diagnostic: CustomDiagnostic = error.into();
+        let diagnostic = CustomDiagnostic::from(&error);
         diagnostic.in_file(file_id)
     }));
 

diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
@@ -1267,8 +1267,11 @@ impl<'block> BrilligBlock<'block> {
         dfg: &DataFlowGraph,
         result_variable: SingleAddrVariable,
     ) {
-        let binary_type =
-            type_of_binary_operation(dfg[binary.lhs].get_type(), dfg[binary.rhs].get_type());
+        let binary_type = type_of_binary_operation(
+            dfg[binary.lhs].get_type(),
+            dfg[binary.rhs].get_type(),
+            binary.operator,
+        );
 
         let left = self.convert_ssa_single_addr_value(binary.lhs, dfg);
         let right = self.convert_ssa_single_addr_value(binary.rhs, dfg);
@@ -1754,7 +1757,7 @@ impl<'block> BrilligBlock<'block> {
 }
 
 /// Returns the type of the operation considering the types of the operands
-pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type) -> Type {
+pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type, op: BinaryOp) -> Type {
     match (lhs_type, rhs_type) {
         (_, Type::Function) | (Type::Function, _) => {
             unreachable!("Functions are invalid in binary operations")
@@ -1770,12 +1773,15 @@ pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type) -> Type
         }
         // If both sides are numeric type, then we expect their types to be
         // the same.
-        (Type::Numeric(lhs_type), Type::Numeric(rhs_type)) => {
+        (Type::Numeric(lhs_type), Type::Numeric(rhs_type))
+            if op != BinaryOp::Shl && op != BinaryOp::Shr =>
+        {
             assert_eq!(
                 lhs_type, rhs_type,
                 "lhs and rhs types in a binary operation are always the same but got {lhs_type} and {rhs_type}"
             );
             Type::Numeric(*lhs_type)
         }
+        _ => lhs_type.clone(),
     }
 }
diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs
@@ -75,12 +75,6 @@ impl BrilligContext {
         result: SingleAddrVariable,
         operation: BrilligBinaryOp,
     ) {
-        assert!(
-            lhs.bit_size == rhs.bit_size,
-            "Not equal bit size for lhs and rhs: lhs {}, rhs {}",
-            lhs.bit_size,
-            rhs.bit_size
-        );
         let is_field_op = lhs.bit_size == FieldElement::max_num_bits();
         let expected_result_bit_size =
             BrilligContext::binary_result_bit_size(operation, lhs.bit_size);

diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
@@ -9,7 +9,7 @@ use noirc_frontend::monomorphization::ast::InlineType;
 use crate::ssa::ir::{
     basic_block::BasicBlockId,
     function::{Function, FunctionId},
-    instruction::{Binary, BinaryOp, Instruction, TerminatorInstruction},
+    instruction::{Binary, BinaryOp, ErrorSelector, Instruction, TerminatorInstruction},
     types::Type,
     value::{Value, ValueId},
 };
@@ -19,7 +19,7 @@ use super::{
         basic_block::BasicBlock,
         dfg::{CallStack, InsertInstructionResult},
         function::RuntimeType,
-        instruction::{ConstrainError, ErrorSelector, ErrorType, InstructionId, Intrinsic},
+        instruction::{ConstrainError, ErrorType, InstructionId, Intrinsic},
     },
     ssa_gen::Ssa,
 };
@@ -232,7 +232,12 @@ impl FunctionBuilder {
     ) -> ValueId {
         let lhs_type = self.type_of_value(lhs);
         let rhs_type = self.type_of_value(rhs);
-        assert_eq!(lhs_type, rhs_type, "ICE - Binary instruction operands must have the same type");
+        if operator != BinaryOp::Shl && operator != BinaryOp::Shr {
+            assert_eq!(
+                lhs_type, rhs_type,
+                "ICE - Binary instruction operands must have the same type"
+            );
+        }
         let instruction = Instruction::Binary(Binary { lhs, rhs, operator });
         self.insert_instruction(instruction, None).first()
     }

diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs
@@ -117,7 +117,7 @@ impl Context<'_> {
         } else {
             // we use a predicate to nullify the result in case of overflow
             let bit_size_var =
-                self.numeric_constant(FieldElement::from(bit_size as u128), typ.clone());
+                self.numeric_constant(FieldElement::from(bit_size as u128), Type::unsigned(8));
             let overflow = self.insert_binary(rhs, BinaryOp::Lt, bit_size_var);
             let predicate = self.insert_cast(overflow, typ.clone());
             // we can safely cast to unsigned because overflow_checks prevent bit-shift with a negative value

diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs
@@ -12,6 +12,8 @@
 //!
 //! Note that this pass also often creates superfluous jmp instructions in the
 //! program that will need to be removed by a later simplify cfg pass.
+//! Note also that unrolling is skipped for Brillig runtime and as a result
+//! we remove reference count instructions because they are only used by Brillig bytecode
 use std::collections::HashSet;
 
 use crate::{
@@ -24,7 +26,7 @@ use crate::{
             dom::DominatorTree,
             function::{Function, RuntimeType},
             function_inserter::FunctionInserter,
-            instruction::TerminatorInstruction,
+            instruction::{Instruction, TerminatorInstruction},
             post_order::PostOrder,
             value::ValueId,
         },
@@ -465,9 +467,14 @@ impl<'f> LoopIteration<'f> {
         // instances of the induction variable or any values that were changed as a result
         // of the new induction variable value.
         for instruction in instructions {
-            self.inserter.push_instruction(instruction, self.insert_block);
+            // Skip reference count instructions since they are only used for brillig, and brillig code is not unrolled
+            if !matches!(
+                self.dfg()[instruction],
+                Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. }
+            ) {
+                self.inserter.push_instruction(instruction, self.insert_block);
+            }
         }
-
         let mut terminator = self.dfg()[self.source_block]
             .unwrap_terminator()
             .clone()

diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
@@ -344,7 +344,7 @@ impl<'a> FunctionContext<'a> {
                         self.insert_safe_cast(result, result_type, location)
                     }
                     BinaryOpKind::ShiftLeft | BinaryOpKind::ShiftRight => {
-                        self.check_shift_overflow(result, rhs, bit_size, location, true)
+                        self.check_shift_overflow(result, rhs, bit_size, location)
                     }
                     _ => unreachable!("operator {} should not overflow", operator),
                 }
@@ -408,7 +408,7 @@ impl<'a> FunctionContext<'a> {
                             }
                         }
 
-                        self.check_shift_overflow(result, rhs, bit_size, location, false);
+                        self.check_shift_overflow(result, rhs, bit_size, location);
                     }
 
                     _ => unreachable!("operator {} should not overflow", operator),
@@ -430,32 +430,12 @@ impl<'a> FunctionContext<'a> {
         rhs: ValueId,
         bit_size: u32,
         location: Location,
-        is_signed: bool,
     ) -> ValueId {
         let one = self.builder.numeric_constant(FieldElement::one(), Type::bool());
-        let rhs = if is_signed {
-            self.insert_safe_cast(rhs, Type::unsigned(bit_size), location)
-        } else {
-            rhs
-        };
-        // Bit-shift with a negative number is an overflow
-        if is_signed {
-            // We compute the sign of rhs.
-            let half_width = self.builder.numeric_constant(
-                FieldElement::from(2_i128.pow(bit_size - 1)),
-                Type::unsigned(bit_size),
-            );
-            let sign = self.builder.insert_binary(rhs, BinaryOp::Lt, half_width);
-            self.builder.set_location(location).insert_constrain(
-                sign,
-                one,
-                Some("attempt to bit-shift with overflow".to_owned().into()),
-            );
-        }
+        assert!(self.builder.current_function.dfg.type_of_value(rhs) == Type::unsigned(8));
 
-        let max = self
-            .builder
-            .numeric_constant(FieldElement::from(bit_size as i128), Type::unsigned(bit_size));
+        let max =
+            self.builder.numeric_constant(FieldElement::from(bit_size as i128), Type::unsigned(8));
         let overflow = self.builder.insert_binary(rhs, BinaryOp::Lt, max);
         self.builder.set_location(location).insert_constrain(
             overflow,

diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs
@@ -40,7 +40,7 @@ pub enum StatementKind {
     Break,
     Continue,
     /// This statement should be executed at compile-time
-    Comptime(Box<StatementKind>),
+    Comptime(Box<Statement>),
     // This is an expression with a trailing semi-colon
     Semi(Expression),
     // This statement is the result of a recovered parse error.
@@ -486,7 +486,8 @@ impl Pattern {
     pub fn name_ident(&self) -> &Ident {
         match self {
             Pattern::Identifier(name_ident) => name_ident,
-            _ => panic!("only the identifier pattern can return a name"),
+            Pattern::Mutable(pattern, ..) => pattern.name_ident(),
+            _ => panic!("Only the Identifier or Mutable patterns can return a name"),
         }
     }
 
@@ -685,7 +686,7 @@ impl Display for StatementKind {
             StatementKind::For(for_loop) => for_loop.fmt(f),
             StatementKind::Break => write!(f, "break"),
             StatementKind::Continue => write!(f, "continue"),
-            StatementKind::Comptime(statement) => write!(f, "comptime {statement}"),
+            StatementKind::Comptime(statement) => write!(f, "comptime {}", statement.kind),
             StatementKind::Semi(semi) => write!(f, "{semi};"),
             StatementKind::Error => write!(f, "Error"),
         }

diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs
@@ -7,7 +7,7 @@ use super::value::Value;
 /// The possible errors that can halt the interpreter.
 #[derive(Debug, Clone)]
 pub enum InterpreterError {
-    ArgumentCountMismatch { expected: usize, actual: usize, call_location: Location },
+    ArgumentCountMismatch { expected: usize, actual: usize, location: Location },
     TypeMismatch { expected: Type, value: Value, location: Location },
     NonComptimeVarReferenced { name: String, location: Location },
     IntegerOutOfRangeForType { value: FieldElement, typ: Type, location: Location },
@@ -60,7 +60,7 @@ impl InterpreterError {
 
     pub fn get_location(&self) -> Location {
         match self {
-            InterpreterError::ArgumentCountMismatch { call_location: location, .. }
+            InterpreterError::ArgumentCountMismatch { location, .. }
             | InterpreterError::TypeMismatch { location, .. }
             | InterpreterError::NonComptimeVarReferenced { location, .. }
             | InterpreterError::IntegerOutOfRangeForType { location, .. }
@@ -98,20 +98,20 @@ impl InterpreterError {
     }
 }
 
-impl From<InterpreterError> for CustomDiagnostic {
-    fn from(error: InterpreterError) -> Self {
+impl<'a> From<&'a InterpreterError> for CustomDiagnostic {
+    fn from(error: &'a InterpreterError) -> Self {
         match error {
-            InterpreterError::ArgumentCountMismatch { expected, actual, call_location } => {
+            InterpreterError::ArgumentCountMismatch { expected, actual, location } => {
                 let only = if expected > actual { "only " } else { "" };
-                let plural = if expected == 1 { "" } else { "s" };
-                let was_were = if actual == 1 { "was" } else { "were" };
+                let plural = if *expected == 1 { "" } else { "s" };
+                let was_were = if *actual == 1 { "was" } else { "were" };
                 let msg = format!(
                     "Expected {expected} argument{plural}, but {only}{actual} {was_were} provided"
                 );
 
                 let few_many = if actual < expected { "few" } else { "many" };
                 let secondary = format!("Too {few_many} arguments");
-                CustomDiagnostic::simple_error(msg, secondary, call_location.span)
+                CustomDiagnostic::simple_error(msg, secondary, location.span)
             }
             InterpreterError::TypeMismatch { expected, value, location } => {
                 let typ = value.get_type();

diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/hir_to_ast.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/hir_to_ast.rs
@@ -67,7 +67,7 @@ impl StmtId {
             HirStatement::Semi(expr) => StatementKind::Semi(expr.to_ast(interner)),
             HirStatement::Error => StatementKind::Error,
             HirStatement::Comptime(statement) => {
-                StatementKind::Comptime(Box::new(statement.to_ast(interner).kind))
+                StatementKind::Comptime(Box::new(statement.to_ast(interner)))
             }
         };