Merge branch 'master' into rk/brillig-check-on-by-default

.github/benchmark_projects.yml

@@ -1,4 +1,4 @@
-define: &AZ_COMMIT a90f08e245add379fa0257c81f8e2819beb190cb
+define: &AZ_COMMIT 966982264a0da23546fb7695079e74b1234da790
 projects:
   private-kernel-inner:
     repo: AztecProtocol/aztec-packages
@@ -35,19 +35,19 @@ projects:
     path: noir-projects/noir-protocol-circuits/crates/rollup-base-private
     num_runs: 5
     timeout: 15
-    compilation-timeout: 10
-    execution-timeout: 0.5
-    compilation-memory-limit: 1100
-    execution-memory-limit: 500
+    compilation-timeout: 20
+    execution-timeout: 1
+    compilation-memory-limit: 1500
+    execution-memory-limit: 650
   rollup-base-public:
     repo: AztecProtocol/aztec-packages
     ref: *AZ_COMMIT
     path: noir-projects/noir-protocol-circuits/crates/rollup-base-public
     num_runs: 5
     timeout: 15
-    compilation-timeout: 8
-    execution-timeout: 0.4
-    compilation-memory-limit: 1000
+    compilation-timeout: 15
+    execution-timeout: 0.75
+    compilation-memory-limit: 1300
     execution-memory-limit: 500
   rollup-block-root-empty:
     repo: AztecProtocol/aztec-packages

EXTERNAL_NOIR_LIBRARIES.yml

@@ -1,4 +1,4 @@
-define: &AZ_COMMIT a90f08e245add379fa0257c81f8e2819beb190cb
+define: &AZ_COMMIT 966982264a0da23546fb7695079e74b1234da790
 libraries:
   noir_check_shuffle:
     repo: noir-lang/noir_check_shuffle

acvm-repo/acir/src/native_types/witness_stack.rs

@@ -12,6 +12,9 @@ use super::WitnessMap;
 enum SerializationError {
     #[error(transparent)]
     Deflate(#[from] std::io::Error),
+
+    #[error(transparent)]
+    BincodeError(#[from] bincode::Error),
 }
 
 #[derive(Debug, Error)]
@@ -57,26 +60,35 @@ impl<F> From<WitnessMap<F>> for WitnessStack<F> {
     }
 }
 
-impl<F: Serialize> TryFrom<WitnessStack<F>> for Vec<u8> {
+impl<F: Serialize> TryFrom<&WitnessStack<F>> for Vec<u8> {
     type Error = WitnessStackError;
 
-    fn try_from(val: WitnessStack<F>) -> Result<Self, Self::Error> {
-        let buf = bincode::serialize(&val).unwrap();
+    fn try_from(val: &WitnessStack<F>) -> Result<Self, Self::Error> {
+        let buf = bincode::serialize(val).map_err(|e| WitnessStackError(e.into()))?;
         let mut deflater = GzEncoder::new(buf.as_slice(), Compression::best());
         let mut buf_c = Vec::new();
         deflater.read_to_end(&mut buf_c).map_err(|err| WitnessStackError(err.into()))?;
         Ok(buf_c)
     }
 }
 
+impl<F: Serialize> TryFrom<WitnessStack<F>> for Vec<u8> {
+    type Error = WitnessStackError;
+
+    fn try_from(val: WitnessStack<F>) -> Result<Self, Self::Error> {
+        Self::try_from(&val)
+    }
+}
+
 impl<F: for<'a> Deserialize<'a>> TryFrom<&[u8]> for WitnessStack<F> {
     type Error = WitnessStackError;
 
     fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
         let mut deflater = GzDecoder::new(bytes);
         let mut buf_d = Vec::new();
         deflater.read_to_end(&mut buf_d).map_err(|err| WitnessStackError(err.into()))?;
-        let witness_stack = bincode::deserialize(&buf_d).unwrap();
+        let witness_stack =
+            bincode::deserialize(&buf_d).map_err(|e| WitnessStackError(e.into()))?;
         Ok(witness_stack)
     }
 }

compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs

@@ -167,6 +167,7 @@ impl Context<'_> {
         let lhs_typ = self.function.dfg.type_of_value(lhs).unwrap_numeric();
         let base = self.field_constant(FieldElement::from(2_u128));
         let pow = self.pow(base, rhs);
+        let pow = self.pow_or_max_for_bit_size(pow, rhs, bit_size, lhs_typ);
         let pow = self.insert_cast(pow, lhs_typ);
         if lhs_typ.is_unsigned() {
             // unsigned right bit shift is just a normal division
@@ -205,6 +206,53 @@ impl Context<'_> {
         }
     }
 
+    /// Returns `pow` or the maximum value allowed for `typ` if 2^rhs is guaranteed to exceed that maximum.
+    fn pow_or_max_for_bit_size(
+        &mut self,
+        pow: ValueId,
+        rhs: ValueId,
+        bit_size: u32,
+        typ: NumericType,
+    ) -> ValueId {
+        let max = if typ.is_unsigned() {
+            if bit_size == 128 { u128::MAX } else { (1_u128 << bit_size) - 1 }
+        } else {
+            1_u128 << (bit_size - 1)
+        };
+        let max = self.field_constant(FieldElement::from(max));
+
+        // Here we check whether rhs is less than the bit_size: if it's not then it will overflow.
+        // Then we do:
+        //
+        // rhs_is_less_than_bit_size = lt rhs, bit_size
+        // rhs_is_not_less_than_bit_size = not rhs_is_less_than_bit_size
+        // pow_when_is_less_than_bit_size = rhs_is_less_than_bit_size * pow
+        // pow_when_is_not_less_than_bit_size = rhs_is_not_less_than_bit_size * max
+        // pow = add pow_when_is_less_than_bit_size, pow_when_is_not_less_than_bit_size
+        //
+        // All operations here are unchecked because they work on field types.
+        let rhs_typ = self.function.dfg.type_of_value(rhs).unwrap_numeric();
+        let bit_size = self.numeric_constant(bit_size as u128, rhs_typ);
+        let rhs_is_less_than_bit_size = self.insert_binary(rhs, BinaryOp::Lt, bit_size);
+        let rhs_is_not_less_than_bit_size = self.insert_not(rhs_is_less_than_bit_size);
+        let rhs_is_less_than_bit_size =
+            self.insert_cast(rhs_is_less_than_bit_size, NumericType::NativeField);
+        let rhs_is_not_less_than_bit_size =
+            self.insert_cast(rhs_is_not_less_than_bit_size, NumericType::NativeField);
+        let pow_when_is_less_than_bit_size =
+            self.insert_binary(rhs_is_less_than_bit_size, BinaryOp::Mul { unchecked: true }, pow);
+        let pow_when_is_not_less_than_bit_size = self.insert_binary(
+            rhs_is_not_less_than_bit_size,
+            BinaryOp::Mul { unchecked: true },
+            max,
+        );
+        self.insert_binary(
+            pow_when_is_less_than_bit_size,
+            BinaryOp::Add { unchecked: true },
+            pow_when_is_not_less_than_bit_size,
+        )
+    }
+
     /// Computes lhs^rhs via square&multiply, using the bits decomposition of rhs
     /// Pseudo-code of the computation:
     /// let mut r = 1;

compiler/noirc_frontend/src/ast/expression.rs

@@ -930,6 +930,13 @@ impl FunctionReturnType {
             FunctionReturnType::Ty(typ) => Cow::Borrowed(typ),
         }
     }
+
+    pub fn location(&self) -> Location {
+        match self {
+            FunctionReturnType::Default(location) => *location,
+            FunctionReturnType::Ty(typ) => typ.location,
+        }
+    }
 }
 
 impl Display for FunctionReturnType {

compiler/noirc_frontend/src/elaborator/mod.rs

@@ -1264,9 +1264,13 @@ impl<'context> Elaborator<'context> {
         self.check_parent_traits_are_implemented(&trait_impl);
         self.remove_trait_impl_assumed_trait_implementations(trait_impl.impl_id);
 
-        for (module, function, _) in &trait_impl.methods.functions {
+        for (module, function, noir_function) in &trait_impl.methods.functions {
             self.local_module = *module;
-            let errors = check_trait_impl_method_matches_declaration(self.interner, *function);
+            let errors = check_trait_impl_method_matches_declaration(
+                self.interner,
+                *function,
+                noir_function,
+            );
             self.push_errors(errors.into_iter().map(|error| error.into()));
         }
 

compiler/noirc_frontend/src/elaborator/statements.rs

@@ -91,15 +91,19 @@ impl Elaborator<'_> {
         let type_contains_unspecified = let_stmt.r#type.contains_unspecified();
         let annotated_type = self.resolve_inferred_type(let_stmt.r#type);
 
+        let pattern_location = let_stmt.pattern.location();
         let expr_location = let_stmt.expression.location;
         let (expression, expr_type) =
             self.elaborate_expression_with_target_type(let_stmt.expression, Some(&annotated_type));
 
         // Require the top-level of a global's type to be fully-specified
         if type_contains_unspecified && global_id.is_some() {
             let expected_type = annotated_type.clone();
-            let error =
-                ResolverError::UnspecifiedGlobalType { location: expr_location, expected_type };
+            let error = ResolverError::UnspecifiedGlobalType {
+                pattern_location,
+                expr_location,
+                expected_type,
+            };
             self.push_err(error);
         }
 
@@ -202,19 +206,18 @@ impl Elaborator<'_> {
         );
 
         // Check that start range and end range have the same types
-        let range_location = start_location.merge(end_location);
         self.unify(&start_range_type, &end_range_type, || TypeCheckError::TypeMismatch {
             expected_typ: start_range_type.to_string(),
             expr_typ: end_range_type.to_string(),
-            expr_location: range_location,
+            expr_location: end_location,
         });
 
         let expected_type = self.polymorphic_integer();
 
         self.unify(&start_range_type, &expected_type, || TypeCheckError::TypeCannotBeUsed {
             typ: start_range_type.clone(),
             place: "for loop",
-            location: range_location,
+            location: start_location,
         });
 
         self.interner.push_definition_type(identifier.id, start_range_type);

compiler/noirc_frontend/src/elaborator/traits.rs

@@ -273,6 +273,7 @@ impl Elaborator<'_> {
 pub(crate) fn check_trait_impl_method_matches_declaration(
     interner: &mut NodeInterner,
     function: FuncId,
+    noir_function: &NoirFunction,
 ) -> Vec<TypeCheckError> {
     let meta = interner.function_meta(&function);
     let modifiers = interner.function_modifiers(&function);
@@ -349,6 +350,8 @@ pub(crate) fn check_trait_impl_method_matches_declaration(
             definition_type,
             method_name,
             &meta.parameters,
+            &meta.return_type,
+            noir_function,
             meta.name.location,
             &trait_info.name.0.contents,
             &mut errors,
@@ -358,11 +361,14 @@ pub(crate) fn check_trait_impl_method_matches_declaration(
     errors
 }
 
+#[allow(clippy::too_many_arguments)]
 fn check_function_type_matches_expected_type(
     expected: &Type,
     actual: &Type,
     method_name: &str,
     actual_parameters: &Parameters,
+    actual_return_type: &FunctionReturnType,
+    noir_function: &NoirFunction,
     location: Location,
     trait_name: &str,
     errors: &mut Vec<TypeCheckError>,
@@ -381,11 +387,16 @@ fn check_function_type_matches_expected_type(
         if params_a.len() == params_b.len() {
             for (i, (a, b)) in params_a.iter().zip(params_b.iter()).enumerate() {
                 if a.try_unify(b, &mut bindings).is_err() {
+                    let parameter_location = noir_function.def.parameters.get(i);
+                    let parameter_location = parameter_location.map(|param| param.typ.location);
+                    let parameter_location =
+                        parameter_location.unwrap_or_else(|| actual_parameters.0[i].0.location());
+
                     errors.push(TypeCheckError::TraitMethodParameterTypeMismatch {
                         method_name: method_name.to_string(),
                         expected_typ: a.to_string(),
                         actual_typ: b.to_string(),
-                        parameter_location: actual_parameters.0[i].0.location(),
+                        parameter_location,
                         parameter_index: i + 1,
                     });
                 }
@@ -395,7 +406,7 @@ fn check_function_type_matches_expected_type(
                 errors.push(TypeCheckError::TypeMismatch {
                     expected_typ: ret_a.to_string(),
                     expr_typ: ret_b.to_string(),
-                    expr_location: location,
+                    expr_location: actual_return_type.location(),
                 });
             }
         } else {

compiler/noirc_frontend/src/elaborator/types.rs

@@ -174,8 +174,8 @@ impl Elaborator<'_> {
 
         if !kind.unifies(&resolved_type.kind()) {
             let expected_typ_err = CompilationError::TypeError(TypeCheckError::TypeKindMismatch {
-                expected_kind: kind.to_string(),
-                expr_kind: resolved_type.kind().to_string(),
+                expected_kind: kind.clone(),
+                expr_kind: resolved_type.kind(),
                 expr_location: location,
             });
             self.push_err(expected_typ_err);
@@ -523,8 +523,8 @@ impl Elaborator<'_> {
                     (Type::Constant(lhs, lhs_kind), Type::Constant(rhs, rhs_kind)) => {
                         if !lhs_kind.unifies(&rhs_kind) {
                             self.push_err(TypeCheckError::TypeKindMismatch {
-                                expected_kind: lhs_kind.to_string(),
-                                expr_kind: rhs_kind.to_string(),
+                                expected_kind: lhs_kind,
+                                expr_kind: rhs_kind,
                                 expr_location: location,
                             });
                             return Type::Error;
@@ -557,8 +557,8 @@ impl Elaborator<'_> {
     fn check_kind(&mut self, typ: Type, expected_kind: &Kind, location: Location) -> Type {
         if !typ.kind().unifies(expected_kind) {
             self.push_err(TypeCheckError::TypeKindMismatch {
-                expected_kind: expected_kind.to_string(),
-                expr_kind: typ.kind().to_string(),
+                expected_kind: expected_kind.clone(),
+                expr_kind: typ.kind(),
                 expr_location: location,
             });
             return Type::Error;

compiler/noirc_frontend/src/hir/def_collector/errors.rs

@@ -77,7 +77,7 @@ pub enum DefCollectorErrorKind {
 impl DefCollectorErrorKind {
     pub fn location(&self) -> Location {
         match self {
-            DefCollectorErrorKind::Duplicate { first_def: ident, .. }
+            DefCollectorErrorKind::Duplicate { second_def: ident, .. }
             | DefCollectorErrorKind::UnresolvedModuleDecl { mod_name: ident, .. }
             | DefCollectorErrorKind::CannotReexportItemWithLessVisibility {
                 item_name: ident,
@@ -160,10 +160,10 @@ impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic {
                     let second_location = second_def.0.location();
                     let mut diag = Diagnostic::simple_error(
                         primary_message,
-                        format!("First {} found here", &typ),
-                        first_location,
+                        format!("Second {} found here", &typ),
+                        second_location,
                     );
-                    diag.add_secondary(format!("Second {} found here", &typ), second_location);
+                    diag.add_secondary(format!("First {} found here", &typ), first_location);
                     diag
                 }
             }