From baa56ad16e43a1baf08b07a421138dacd05bec47 Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Thu, 3 Aug 2023 15:13:53 +0300 Subject: [PATCH 1/4] fix: don't allow lambda reassignment if closure types dont match --- crates/noirc_frontend/src/hir/type_check/stmt.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/noirc_frontend/src/hir/type_check/stmt.rs b/crates/noirc_frontend/src/hir/type_check/stmt.rs index 3130a8616de..92b962edfac 100644 --- a/crates/noirc_frontend/src/hir/type_check/stmt.rs +++ b/crates/noirc_frontend/src/hir/type_check/stmt.rs @@ -116,6 +116,17 @@ impl<'interner> TypeChecker<'interner> { source: Source::Assignment, } }); + + if let (Type::Function(_, _, env_a), Type::Function(_, _, env_b)) = (&lvalue_type, &expr_type) { + env_a.unify(&env_b, span, &mut self.errors, || { + TypeCheckError::TypeMismatchWithSource { + rhs: expr_type.clone(), + lhs: lvalue_type.clone(), + span, + source: Source::Assignment, + } + }); + } } /// Type check an lvalue - the left hand side of an assignment statement. From a377e274b613db4150afd0734c3fa1c0c958f5f1 Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Fri, 4 Aug 2023 07:49:24 +0300 Subject: [PATCH 2/4] fix: add test for bad lambda reassignment --- .../fail/different-lambda-env-reassign-disallow.nr | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 crates/nargo_cli/tests/compile_tests_data/fail/different-lambda-env-reassign-disallow.nr diff --git a/crates/nargo_cli/tests/compile_tests_data/fail/different-lambda-env-reassign-disallow.nr b/crates/nargo_cli/tests/compile_tests_data/fail/different-lambda-env-reassign-disallow.nr new file mode 100644 index 00000000000..7bac2367846 --- /dev/null +++ b/crates/nargo_cli/tests/compile_tests_data/fail/different-lambda-env-reassign-disallow.nr @@ -0,0 +1,9 @@ +fn bad() { + let a: i32 = 100; + let b: i32 = 200; + + let mut f = || a; + + // this should fail with a type error, since the closures have different environments & types + f = || a + b; +} \ No newline at end of file From c6e4d883d34d3bc8eb003fd2e81b147bff3e5d52 Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Fri, 4 Aug 2023 07:57:34 +0300 Subject: [PATCH 3/4] fix: cargo fmt & clippy --- crates/noirc_frontend/src/hir/type_check/stmt.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/noirc_frontend/src/hir/type_check/stmt.rs b/crates/noirc_frontend/src/hir/type_check/stmt.rs index 92b962edfac..e0c03d465d0 100644 --- a/crates/noirc_frontend/src/hir/type_check/stmt.rs +++ b/crates/noirc_frontend/src/hir/type_check/stmt.rs @@ -117,14 +117,14 @@ impl<'interner> TypeChecker<'interner> { } }); - if let (Type::Function(_, _, env_a), Type::Function(_, _, env_b)) = (&lvalue_type, &expr_type) { - env_a.unify(&env_b, span, &mut self.errors, || { - TypeCheckError::TypeMismatchWithSource { - rhs: expr_type.clone(), - lhs: lvalue_type.clone(), - span, - source: Source::Assignment, - } + if let (Type::Function(_, _, env_a), Type::Function(_, _, env_b)) = + (&lvalue_type, &expr_type) + { + env_a.unify(env_b, span, &mut self.errors, || TypeCheckError::TypeMismatchWithSource { + rhs: expr_type.clone(), + lhs: lvalue_type.clone(), + span, + source: Source::Assignment, }); } } From dfc0ba0e16267b48f0d11915b71329a9adb870bf Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Mon, 7 Aug 2023 07:54:41 +0300 Subject: [PATCH 4/4] fix: alternate approach --- crates/noirc_frontend/src/hir/type_check/stmt.rs | 11 ----------- crates/noirc_frontend/src/hir_def/types.rs | 8 ++++++-- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/crates/noirc_frontend/src/hir/type_check/stmt.rs b/crates/noirc_frontend/src/hir/type_check/stmt.rs index e0c03d465d0..3130a8616de 100644 --- a/crates/noirc_frontend/src/hir/type_check/stmt.rs +++ b/crates/noirc_frontend/src/hir/type_check/stmt.rs @@ -116,17 +116,6 @@ impl<'interner> TypeChecker<'interner> { source: Source::Assignment, } }); - - if let (Type::Function(_, _, env_a), Type::Function(_, _, env_b)) = - (&lvalue_type, &expr_type) - { - env_a.unify(env_b, span, &mut self.errors, || TypeCheckError::TypeMismatchWithSource { - rhs: expr_type.clone(), - lhs: lvalue_type.clone(), - span, - source: Source::Assignment, - }); - } } /// Type check an lvalue - the left hand side of an assignment statement. diff --git a/crates/noirc_frontend/src/hir_def/types.rs b/crates/noirc_frontend/src/hir_def/types.rs index d77b8033ba1..587001bfafc 100644 --- a/crates/noirc_frontend/src/hir_def/types.rs +++ b/crates/noirc_frontend/src/hir_def/types.rs @@ -1206,12 +1206,14 @@ impl Type { } } - (Function(params_a, ret_a, _env_a), Function(params_b, ret_b, _env_b)) => { + (Function(params_a, ret_a, env_a), Function(params_b, ret_b, env_b)) => { if params_a.len() == params_b.len() { for (a, b) in params_a.iter().zip(params_b.iter()) { a.try_unify(b, span)?; } + env_a.try_unify(env_b, span)?; + ret_b.try_unify(ret_a, span) } else { Err(SpanKind::None) @@ -1413,12 +1415,14 @@ impl Type { } } - (Function(params_a, ret_a, _env_a), Function(params_b, ret_b, _env_b)) => { + (Function(params_a, ret_a, env_a), Function(params_b, ret_b, env_b)) => { if params_a.len() == params_b.len() { for (a, b) in params_a.iter().zip(params_b) { a.is_subtype_of(b, span)?; } + env_a.is_subtype_of(env_b, span)?; + // return types are contravariant, so this must be ret_b <: ret_a instead of the reverse ret_b.is_subtype_of(ret_a, span) } else {