Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiler panics when trying to create a mutable reference of an undefined variable #1985

Closed
iAmMichaelConnor opened this issue Jul 20, 2023 · 4 comments · Fixed by #1987
Closed
Labels
bug Something isn't working

Comments

@iAmMichaelConnor
Copy link
Collaborator

iAmMichaelConnor commented Jul 20, 2023

This isn't blocking anything, so not a high priority. It's just something I stumbled upon.

fn my_mutating_fn(a: &mut Field) {
    a += 10;
}

fn main(mut a: Field) -> pub Field {
    my_mutating_fn(&mut a);
    a
}
image

I'd have expected + to work here.

Compiled using the latest master, from source.

@iAmMichaelConnor iAmMichaelConnor added the bug Something isn't working label Jul 20, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Jul 20, 2023
@ghost
Copy link

ghost commented Jul 20, 2023

looks like you forgot to deref, but still get error in noir

fn my_mutating_fn(a: &mut Field) {
    *a += 10;
}

fn main(mut x: Field) -> pub Field {
    my_mutating_fn(&mut a);
    a
}
$ nargo execute --show-ssa --experimental-ssa
The application panicked (crashed).
Message:  index out of bounds: the len is 1254 but the index is 18446744073709551615
Location: crates/noirc_frontend/src/node_interner.rs:482

This is a bug. We may have already fixed this in newer versions of Nargo so try searching for similar issues at https://github.com/noir-lang/noir/issues/.
If there isn't an open issue for this bug, consider opening one at https://github.com/noir-lang/noir/issues/new?labels=bug&template=bug_report.yml

@iAmMichaelConnor
Copy link
Collaborator Author

Doh! Good catch.
Your error is even better!

@jfecher
Copy link
Contributor

jfecher commented Jul 20, 2023

Thanks for the crash report, I've fixed it locally and will have a PR up soon. The issue is that in main you're using a which was not declared and the mutable reference code did not have a check for this case. Since it needs to check a variable's definition to see if it is mutable and a had no definition. Here's the new error messages with the fix:

error: cannot find `a` in this scope 
  ┌─ /.../src/main.nr:8:25
  │
8 │     my_mutating_fn(&mut a);
  │                         - not found in this scope

error: cannot find `a` in this scope 
  ┌─ /.../src/main.nr:9:5
  │
9 │     a
  │     - not found in this scope

warning: unused variable x
  ┌─ /.../src/main.nr:7:13
  │
7 │ fn main(mut x: Field) -> pub Field {
  │             - unused variable 

@jfecher jfecher changed the title Can't perform binary operation on &mut Field and Field Compiler panics when trying to create a mutable reference of an undefined variable Jul 20, 2023
@jfecher
Copy link
Contributor

jfecher commented Jul 20, 2023

Looking at the original code again, it uses a everywhere and doesn't have the x parameter. So it was an uncaught mistake while copying that lead to catching a compiler bug.

@iAmMichaelConnor I would expect your code to work then with only adding the one dereference. Note that you need this dereference because, like in rust, references are separate from their element types. So you cannot add together a &mut Field and a Field. You also cannot assign to a &mut Field directly, oddly enough. You have to dereference it to signal that you're storing the Field in that reference, rather than mutating that reference to be a different reference.

@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants