Skip to content

Commit

Permalink
fix: Clean error when attemping to return a slice from Brillig to ACIR (
Browse files Browse the repository at this point in the history
#4280)

# 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:
<img width="811" alt="Screenshot 2024-02-06 at 1 33 39 PM"
src="https://github.com/noir-lang/noir/assets/43554004/e37e7b3b-13ef-4222-a359-2f14084478da">


## 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>
  • Loading branch information
vezenovm and TomAFrench authored Feb 6, 2024
1 parent e5cfb4d commit bcad4ec
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 1 deletion.
16 changes: 15 additions & 1 deletion compiler/noirc_evaluator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
}
}
}
Expand Down Expand Up @@ -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 =
Expand Down
8 changes: 8 additions & 0 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "brillig_slice_to_acir"
type = "bin"
authors = [""]
compiler_version = ">=0.23.0"

[dependencies]
14 changes: 14 additions & 0 deletions test_programs/compile_failure/brillig_slice_to_acir/src/main.nr
Original file line number Diff line number Diff line change
@@ -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
}
7 changes: 7 additions & 0 deletions test_programs/compile_failure/brillig_vec_to_acir/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "brillig_vec_to_acir"
type = "bin"
authors = [""]
compiler_version = ">=0.23.0"

[dependencies]
14 changes: 14 additions & 0 deletions test_programs/compile_failure/brillig_vec_to_acir/src/main.nr
Original file line number Diff line number Diff line change
@@ -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<u32> {
let mut a = Vec::new();
for i in 0..y {
a.push(x[i]);
}
a
}

0 comments on commit bcad4ec

Please sign in to comment.