From f45603687ee1e277fd0b18effe23afbe7aabb0ad Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 14 Aug 2024 15:33:17 +0000 Subject: [PATCH 1/6] continue type checking when unsafe block is missing --- .../src/elaborator/expressions.rs | 2 +- compiler/noirc_frontend/src/elaborator/mod.rs | 2 +- .../noirc_frontend/src/elaborator/types.rs | 1 - compiler/noirc_frontend/src/tests.rs | 42 +++++++++++++++++++ 4 files changed, 44 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 5611c3ce0d0..41e44d724e8 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -114,7 +114,7 @@ impl<'context> Elaborator<'context> { self.in_unsafe_block = true; let (hir_block_expression, typ) = self.elaborate_block_expression(block); - + // Finally, we restore the original value of `self.in_unsafe_block`. self.in_unsafe_block = old_in_unsafe_block; diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index da35ca77316..c4e49415765 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -425,7 +425,7 @@ impl<'context> Elaborator<'context> { (HirFunction::unchecked_from_expr(expr_id), body_type) } }; - + // Don't verify the return type for builtin functions & trait function declarations if !func_meta.is_stub() { self.type_check_function_body(body_type, &func_meta, hir_func.as_expr()); diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 1f43bdf3ecd..7fb1f59a97d 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -1328,7 +1328,6 @@ impl<'context> Elaborator<'context> { if crossing_runtime_boundary { if !self.in_unsafe_block { self.push_err(TypeCheckError::Unsafe { span }); - return Type::Error; } let called_func_id = self diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index ba39980a0ef..d529f02de46 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2495,6 +2495,48 @@ fn cannot_call_unconstrained_first_class_function_outside_of_unsafe() { }; } +#[test] +fn missing_unsafe_block_when_needing_type_annotations() { + // This test is a regression check that even when an unsafe block is missing + // that we still appropriately continue type checking and infer type annotations. + let src = r#" + fn main() { + let z = BigNum { limbs: [2, 0, 0] }; + println(z.__is_zero()); + } + + struct BigNum { + limbs: [u64; N], + } + + impl BigNum { + unconstrained fn __is_zero_impl(self) -> bool { + let mut result: bool = true; + for i in 0..N { + result = result & (self.limbs[i] == 0); + } + result + } + } + + trait BigNumTrait { + fn __is_zero(self) -> bool; + } + + impl BigNumTrait for BigNum { + fn __is_zero(self) -> bool { + self.__is_zero_impl() + } + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::TypeError(TypeCheckError::Unsafe { .. }) = &errors[0].0 else { + panic!("Expected an 'unsafe' error, got {:?}", errors[0].0); + }; +} + #[test] fn cannot_pass_unconstrained_function_to_regular_function() { let src = r#" From 6466f27c640076fd8154d8a4a22b29a71bec4b8f Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 14 Aug 2024 15:33:30 +0000 Subject: [PATCH 2/6] cargo fmt --- compiler/noirc_frontend/src/elaborator/expressions.rs | 2 +- compiler/noirc_frontend/src/elaborator/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 41e44d724e8..5611c3ce0d0 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -114,7 +114,7 @@ impl<'context> Elaborator<'context> { self.in_unsafe_block = true; let (hir_block_expression, typ) = self.elaborate_block_expression(block); - + // Finally, we restore the original value of `self.in_unsafe_block`. self.in_unsafe_block = old_in_unsafe_block; diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index c4e49415765..da35ca77316 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -425,7 +425,7 @@ impl<'context> Elaborator<'context> { (HirFunction::unchecked_from_expr(expr_id), body_type) } }; - + // Don't verify the return type for builtin functions & trait function declarations if !func_meta.is_stub() { self.type_check_function_body(body_type, &func_meta, hir_func.as_expr()); From 104be55643f12607d325db65c0929df14051c4cf Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 14 Aug 2024 15:37:54 +0000 Subject: [PATCH 3/6] switch to assert in test --- compiler/noirc_frontend/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index d529f02de46..a39e3a8ff20 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2502,7 +2502,7 @@ fn missing_unsafe_block_when_needing_type_annotations() { let src = r#" fn main() { let z = BigNum { limbs: [2, 0, 0] }; - println(z.__is_zero()); + assert(z.__is_zero() == false); } struct BigNum { From 160d9989276f542ef04d38f20ccce02170fa2988 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 14 Aug 2024 16:16:49 +0000 Subject: [PATCH 4/6] fix lookup_function_from_expr to account for local exprs --- compiler/noirc_frontend/src/elaborator/types.rs | 2 +- compiler/noirc_frontend/src/node_interner.rs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 7fb1f59a97d..cb19d34bb6d 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -17,7 +17,7 @@ use crate::{ }, hir_def::{ expr::{ - HirBinaryOp, HirCallExpression, HirMemberAccess, HirMethodReference, + HirBinaryOp, HirCallExpression, HirIdent, HirMemberAccess, HirMethodReference, HirPrefixExpression, }, function::{FuncMeta, Parameters}, diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 9cffebb0cee..e9d1dbb67f8 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -951,12 +951,12 @@ impl NodeInterner { /// Returns the [`FuncId`] corresponding to the function referred to by `expr_id` pub fn lookup_function_from_expr(&self, expr: &ExprId) -> Option { if let HirExpression::Ident(HirIdent { id, .. }, _) = self.expression(expr) { - if let Some(DefinitionKind::Function(func_id)) = - self.try_definition(id).map(|def| &def.kind) - { - Some(*func_id) - } else { - None + match self.try_definition(id).map(|def| &def.kind) { + Some(DefinitionKind::Function(func_id)) => Some(*func_id), + Some(DefinitionKind::Local(Some(expr_id))) => { + self.lookup_function_from_expr(expr_id) + } + _ => None, } } else { None From de2ae5be7f758026ee7a959f6a712fe11f314939 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 14 Aug 2024 16:31:28 +0000 Subject: [PATCH 5/6] use if let for lookup_function_from_expr --- .../noirc_frontend/src/elaborator/types.rs | 32 +++++++++++-------- compiler/noirc_frontend/src/tests.rs | 17 +++++++--- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index cb19d34bb6d..478364635af 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -17,7 +17,7 @@ use crate::{ }, hir_def::{ expr::{ - HirBinaryOp, HirCallExpression, HirIdent, HirMemberAccess, HirMethodReference, + HirBinaryOp, HirCallExpression, HirMemberAccess, HirMethodReference, HirPrefixExpression, }, function::{FuncMeta, Parameters}, @@ -1330,19 +1330,23 @@ impl<'context> Elaborator<'context> { self.push_err(TypeCheckError::Unsafe { span }); } - let called_func_id = self - .interner - .lookup_function_from_expr(&call.func) - .expect("Called function should exist"); - self.run_lint(|elaborator| { - lints::oracle_called_from_constrained_function( - elaborator.interner, - &called_func_id, - is_current_func_constrained, - span, - ) - .map(Into::into) - }); + // let called_func_id = self + // .interner + // .lookup_function_from_expr(&call.func) + // .expect("Called function should exist"); + + if let Some(called_func_id) = self.interner.lookup_function_from_expr(&call.func) { + self.run_lint(|elaborator| { + lints::oracle_called_from_constrained_function( + elaborator.interner, + &called_func_id, + is_current_func_constrained, + span, + ) + .map(Into::into) + }); + } + let errors = lints::unconstrained_function_args(&args); for error in errors { self.push_err(error); diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index a39e3a8ff20..26a82216eee 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2482,17 +2482,26 @@ fn cannot_call_unconstrained_first_class_function_outside_of_unsafe() { let src = r#" fn main() { let func = foo; + // Warning should trigger here func(); + inner(func); + } + + fn inner(x: unconstrained fn() -> ()) { + // Warning should trigger here + x(); } unconstrained fn foo() {} "#; let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); + assert_eq!(errors.len(), 2); - let CompilationError::TypeError(TypeCheckError::Unsafe { .. }) = &errors[0].0 else { - panic!("Expected an 'unsafe' error, got {:?}", errors[0].0); - }; + for error in &errors { + let CompilationError::TypeError(TypeCheckError::Unsafe { .. }) = &error.0 else { + panic!("Expected an 'unsafe' error, got {:?}", errors[0].0); + }; + } } #[test] From 7f3a69c691f3e6046816cdb829d04b2523e9bd55 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 14 Aug 2024 16:31:41 +0000 Subject: [PATCH 6/6] cleanup --- compiler/noirc_frontend/src/elaborator/types.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 478364635af..58d019c86aa 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -1330,11 +1330,6 @@ impl<'context> Elaborator<'context> { self.push_err(TypeCheckError::Unsafe { span }); } - // let called_func_id = self - // .interner - // .lookup_function_from_expr(&call.func) - // .expect("Called function should exist"); - if let Some(called_func_id) = self.interner.lookup_function_from_expr(&call.func) { self.run_lint(|elaborator| { lints::oracle_called_from_constrained_function(