From bcad4ec5cc3e3f606e5bf673c7e367f1b63b20a2 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 6 Feb 2024 19:24:54 +0000 Subject: [PATCH] fix: Clean error when attemping to return a slice from Brillig to ACIR (#4280) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description ## Problem\* Resolves #2535 ## Summary\* We currently panic when attempting to return a slice from an unconstrained runtime to an ACIR runtime. In some cases such as in the linked issue the panic is not very clear at all. New error: Screenshot 2024-02-06 at 1 33 39 PM ## Additional Context ## Documentation\* Check one: - [ ] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [ ] I have tested the changes locally. - [ ] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- compiler/noirc_evaluator/src/errors.rs | 16 +++++++++++++++- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 8 ++++++++ .../brillig_slice_to_acir/Nargo.toml | 7 +++++++ .../brillig_slice_to_acir/src/main.nr | 14 ++++++++++++++ .../brillig_vec_to_acir/Nargo.toml | 7 +++++++ .../brillig_vec_to_acir/src/main.nr | 14 ++++++++++++++ 6 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 test_programs/compile_failure/brillig_slice_to_acir/Nargo.toml create mode 100644 test_programs/compile_failure/brillig_slice_to_acir/src/main.nr create mode 100644 test_programs/compile_failure/brillig_vec_to_acir/Nargo.toml create mode 100644 test_programs/compile_failure/brillig_vec_to_acir/src/main.nr diff --git a/compiler/noirc_evaluator/src/errors.rs b/compiler/noirc_evaluator/src/errors.rs index 020a807cb67..f6f404204f5 100644 --- a/compiler/noirc_evaluator/src/errors.rs +++ b/compiler/noirc_evaluator/src/errors.rs @@ -46,6 +46,8 @@ pub enum RuntimeError { NestedSlice { call_stack: CallStack }, #[error("Big Integer modulus do no match")] BigIntModulus { call_stack: CallStack }, + #[error("Slices cannot be returned from an unconstrained runtime to a constrained runtime")] + UnconstrainedSliceReturnToConstrained { call_stack: CallStack }, } // We avoid showing the actual lhs and rhs since most of the time they are just 0 @@ -135,7 +137,8 @@ impl RuntimeError { | RuntimeError::IntegerOutOfBounds { call_stack, .. } | RuntimeError::UnsupportedIntegerSize { call_stack, .. } | RuntimeError::NestedSlice { call_stack, .. } - | RuntimeError::BigIntModulus { call_stack, .. } => call_stack, + | RuntimeError::BigIntModulus { call_stack, .. } + | RuntimeError::UnconstrainedSliceReturnToConstrained { call_stack } => call_stack, } } } @@ -171,6 +174,17 @@ impl RuntimeError { location.span, ) } + RuntimeError::UnconstrainedSliceReturnToConstrained { .. } => { + let primary_message = self.to_string(); + let location = + self.call_stack().back().expect("Expected RuntimeError to have a location"); + + Diagnostic::simple_error( + primary_message, + "If attempting to return a `Vec` type, `Vec` contains a slice internally.".to_string(), + location.span, + ) + } _ => { let message = self.to_string(); let location = diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 03b65d513a6..337bf2608b5 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -509,6 +509,14 @@ impl Context { "expected an intrinsic/brillig call, but found {func:?}. All ACIR methods should be inlined" ), RuntimeType::Brillig => { + // Check that we are not attempting to return a slice from + // an unconstrained runtime to a constrained runtime + for result_id in result_ids { + if dfg.type_of_value(*result_id).contains_slice_element() { + return Err(RuntimeError::UnconstrainedSliceReturnToConstrained { call_stack: self.acir_context.get_call_stack() }) + } + } + let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg)); let code = self.gen_brillig_for(func, brillig)?; diff --git a/test_programs/compile_failure/brillig_slice_to_acir/Nargo.toml b/test_programs/compile_failure/brillig_slice_to_acir/Nargo.toml new file mode 100644 index 00000000000..c3e51561cc7 --- /dev/null +++ b/test_programs/compile_failure/brillig_slice_to_acir/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "brillig_slice_to_acir" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_failure/brillig_slice_to_acir/src/main.nr b/test_programs/compile_failure/brillig_slice_to_acir/src/main.nr new file mode 100644 index 00000000000..dcf23aac5f5 --- /dev/null +++ b/test_programs/compile_failure/brillig_slice_to_acir/src/main.nr @@ -0,0 +1,14 @@ +global DEPTH: Field = 40000; + +fn main(x: [u32; DEPTH], y: u32) { + let mut new_x = []; + new_x = clear(x, y); +} + +unconstrained fn clear(x: [u32; DEPTH], y: u32) -> [u32] { + let mut a = []; + for i in 0..y { + a = a.push_back(x[i]); + } + a +} diff --git a/test_programs/compile_failure/brillig_vec_to_acir/Nargo.toml b/test_programs/compile_failure/brillig_vec_to_acir/Nargo.toml new file mode 100644 index 00000000000..c09fc417b55 --- /dev/null +++ b/test_programs/compile_failure/brillig_vec_to_acir/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "brillig_vec_to_acir" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_failure/brillig_vec_to_acir/src/main.nr b/test_programs/compile_failure/brillig_vec_to_acir/src/main.nr new file mode 100644 index 00000000000..8f872f1b903 --- /dev/null +++ b/test_programs/compile_failure/brillig_vec_to_acir/src/main.nr @@ -0,0 +1,14 @@ +global DEPTH: Field = 40000; + +fn main(x: [u32; DEPTH], y: u32) { + let mut new_x = Vec::new(); + new_x = clear(x, y); +} + +unconstrained fn clear(x: [u32; DEPTH], y: u32) -> Vec { + let mut a = Vec::new(); + for i in 0..y { + a.push(x[i]); + } + a +}