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

Arrays of mutable references work in tests, but sometimes panic on compile #5175

Closed
Thjarkgh opened this issue Jun 5, 2024 · 1 comment · Fixed by #5200
Closed

Arrays of mutable references work in tests, but sometimes panic on compile #5175

Thjarkgh opened this issue Jun 5, 2024 · 1 comment · Fixed by #5200
Labels
bug Something isn't working

Comments

@Thjarkgh
Copy link

Thjarkgh commented Jun 5, 2024

Aim

The actual use case is more complicated than the snippet I post below. I want to pass on a list of mutable characters (which all have various attributes like position, health etc.) to various functions that handle player actions and events. I would like to avoid a syntax like:

fn do_something(char: &mut Character) {
   // modify char
}
//...
let mut my_char_0 = Character {...};
let mut my_char_1 = Character {...};
///...
if actor_id == 0 { do_something(&mut my_char_0); }
else if actor_id == 1 { do_something(&mut my_char_1);
// ...

I would also prefer to avoid creating hard-to-test monster classes like

struct Game { my_chars, their_chars, map, traps, their_traps, .... }
impl Game { fn attack(&mut self, ...) {} fn move(&mut self, ...) {} fn see(&mut self, ...) {} ....

I would rather prefer being able to pass on the whole array, however, since array elements cannot be referenced and non-mutable-references that are being passed on are only local, my idea was to use arrays of mutable references. This allows the following syntax:

let my_chars = [&mut Character {...}, &mut Character {...}, ...];
do_something(my_chars[actor_id]);

These references could then even be stored in the Game struct
In tests this works all fine, however, when trying to build (nargo compile), I get an error

Expected Behavior

If Noir language server and nargo test accept a syntax, I expect it to work on nargo compile as well.

Instead the compiler panics.

Bug

The application panicked (crashed).
Message:  internal error: entered unreachable code: Expected all allocate instructions to be removed before acir_gen
Location: compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs:689

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

To Reproduce

The minimal version to reproduce this bug is the following code:

fn main(actor_id: u8, class: pub u8) {
    assert(actor_id < 5);
    let mut game = [&mut 1, &mut 1, &mut 5, &mut 1, &mut 1];
    assert(*game[actor_id] == class);
}

#[test]
fn test_main() {
    main(2, 5);
}

Project Impact

None

Impact Context

No response

Workaround

Yes

Workaround Description

The workaround is to use gigantic return values (in the simple example below this is not too bad, but imagine the Game struct having more properties ...)

struct Character {
    x: u8,
    y: u8,
    health: u8,
}

struct Game {
    my_chars: [Character; 5],
}

fn handle_explosion(game: Game, x: u8, y: u8, dmg: u8) -> Game {
    let mut new_chars = game.my_chars;
    for i in 0..5 {
        if (new_chars[i].x == x) & (new_chars[i].y == y) {
            new_chars[i] = Character { x: x, y: y, health: new_chars[i].health - dmg };
        } else if ((new_chars[i].x == x) & ((new_chars[i].y == y + 1) | (new_chars[i].y == y - 1)))
            | ((new_chars[i].y == y) & ((new_chars[i].x == x + 1) | (new_chars[i].x == x - 1))) {
            new_chars[i] = Character { x: x, y: y, health: new_chars[i].health - (dmg / 2) };
        }
    }
    Game { my_chars: new_chars }
}

fn main(my_chars: [Character; 5], explosion_x: pub u8, explosion_y: pub u8, healths: pub [u8; 5]) {
    let game = Game { my_chars };
    let new_game = handle_explosion(game, explosion_x, explosion_y, 100);
    for i in 0..5 {
        assert(new_game.my_chars[i].health == healths[i]);
    }
}

#[test]
fn test_main() {
    main(
        [
        Character { x: 12, y: 5, health: 100 },
        Character { x: 13, y: 5, health: 100 },
        Character { x: 13, y: 4, health: 100 },
        Character { x: 20, y: 5, health: 100 },
        Character { x: 20, y: 0, health: 100 }
    ],
        13,
        5,
        [50, 0, 50, 100, 100]
    );
}

or god-classes:

struct Character {
    x: u8,
    y: u8,
    health: u8,
}

struct Game {
    my_chars: [Character; 5],
}

impl Game {
    fn handle_explosion(&mut self, x: u8, y: u8, dmg: u8) {
        for i in 0..5 {
            if (self.my_chars[i].x == x) & (self.my_chars[i].y == y) {
                self.my_chars[i] = Character { x: x, y: y, health: self.my_chars[i].health - dmg };
            } else if ((self.my_chars[i].x == x)
                & ((self.my_chars[i].y == y + 1) | (self.my_chars[i].y == y - 1)))
                | ((self.my_chars[i].y == y)
                    & ((self.my_chars[i].x == x + 1) | (self.my_chars[i].x == x - 1))) {
                self.my_chars[i] = Character { x: x, y: y, health: self.my_chars[i].health - (dmg / 2) };
            }
        }
    }
}

fn main(my_chars: [Character; 5], explosion_x: pub u8, explosion_y: pub u8, healths: pub [u8; 5]) {
    let mut game = Game { my_chars };
    game.handle_explosion(explosion_x, explosion_y, 100);
    for i in 0..5 {
        assert(game.my_chars[i].health == healths[i]);
    }
}

#[test]
fn test_main() {
    main(
        [
        Character { x: 12, y: 5, health: 100 },
        Character { x: 13, y: 5, health: 100 },
        Character { x: 13, y: 4, health: 100 },
        Character { x: 20, y: 5, health: 100 },
        Character { x: 20, y: 0, health: 100 }
    ],
        13,
        5,
        [50, 0, 50, 100, 100]
    );
}

Additional Context

Interestingly, sometimes the bug does not appear. Like here:

struct Character {
    x: u8,
    y: u8,
    health: u8,
}

impl Character {
    fn take_damage(&mut self, dmg: u8) {
        if self.health < dmg {
            self.health = 0;
        } else {
            self.health = self.health - dmg;
        }
    }
}

struct Game {
    my_chars: [&mut Character; 5],
}

impl Game {
    fn build(mut char_0: Character, mut char_1: Character, mut char_2: Character, mut char_3: Character, mut char_4: Character) -> Game {
        Game {
            my_chars: [
                &mut char_0,
                &mut char_1,
                &mut char_2,
                &mut char_3,
                &mut char_4,
            ],
        }
    }
}

fn handle_explosion(my_chars: [&mut Character; 5], x: u8, y: u8, dmg: u8) {
    for i in 0..5 {
        if (my_chars[i].x == x) & (my_chars[i].y == y) {
            my_chars[i].take_damage(dmg);
        } else if ((my_chars[i].x == x)
            & ((my_chars[i].y == y + 1) | (my_chars[i].y == y - 1)))
            | ((my_chars[i].y == y)
                & ((my_chars[i].x == x + 1) | (my_chars[i].x == x - 1))) {
            my_chars[i].take_damage(dmg / 2);
        }
    }
}

fn main(mut char_0: Character, mut char_1: Character, mut char_2: Character, mut char_3: Character, mut char_4: Character, explosion_x: pub u8, explosion_y: pub u8, healths: pub [u8; 5]) {
    let game = Game::build(char_0, char_1, char_2, char_3, char_4);
    handle_explosion(game.my_chars, explosion_x, explosion_y, 100);
    for i in 0..5 {
        assert(game.my_chars[i].health == healths[i]);
    }
}

#[test]
fn test_main() {
    main(
        Character { x: 12, y: 5, health: 100 },
        Character { x: 13, y: 5, health: 100 },
        Character { x: 13, y: 4, health: 100 },
        Character { x: 20, y: 5, health: 100 },
        Character { x: 20, y: 0, health: 100 },
        13,
        5,
        [50, 0, 50, 100, 100]
    );
}

Installation Method

Binary (noirup default)

Nargo Version

nargo version = 0.30.0 noirc version = 0.30.0+af57471035e4fa7eaffa71693219df6d029dbcde (git version hash: af57471, is dirty: false)

NoirJS Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@Thjarkgh Thjarkgh added the bug Something isn't working label Jun 5, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Jun 5, 2024
@guipublic
Copy link
Contributor

The error you are getting is due to the fact we do not handle references which we cannot resolve at compile time.
This is why in some cases you do not get the issue; when the references could be resolved.

I made a PR to report this error instead of the compiler panic.

github-merge-queue bot pushed a commit that referenced this issue Jun 10, 2024
# Description

## Problem\*

Resolves #5175 

## Summary\*
When we cannot resolve some references to an array, its allocate is not
simplified and we get a panic.
I changed this to an error message saying that we could not resolve all
references from the array.
I believe an error message is better than the panic, however I am not
sure whether having remaining allocates only happens because of this
case.


## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** 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.
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Jun 10, 2024
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
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants