Internal compiler error: Verification failed: Struct access must be to a pointer value, not a { string<10> }

[Braqzen]

The following code produces an ICE when ref is removed from the impl method.

contract;

struct Todo {
    text: str[10]
}

impl Todo {
    fn update_text(ref mut self, text: str[10]) {
        self.text = text;
    }
}

storage {
    todo: Todo = Todo {
        text: "1234567890",
    }
}

abi Todos {
    #[storage(read, write)]
    fn update_text(text: str[10]);
}

impl Todos for Contract {
    #[storage(read, write)]
    fn update_text(text: str[10]) {
        storage.todo.update_text(text);
    }
}

Note that there was a fuel-toolchain.toml file at the time of writing this issue and all it contained is

[toolchain]
channel = "beta-3"

[tritao]

Smaller repro:

script;

struct S {
    data: u64
}

impl S {
    fn update(mut self, data: u64) {
        self.data = data
    }
}

fn main() {
  let mut s = S { data: 0 };
  s.update(10);
}

@mohammadfawaz Should we disallow the usage mut self and always require it to be ref in such cases?

[mohammadfawaz]

@tritao does the problem also affect mut struct arguments that are not self?

[tritao]

@tritao does the problem also affect mut struct arguments that are not self?

So, the following fails:

script;

struct S {
  data: u64
}

impl S {
  fn update(self, mut data: u64) {
    data = 20
  }
}

fn main() {
  let s = S { data: 0 };
  s.update(10);
}

With current master (after IR refactor):

error: Internal compiler error: Store must be to a pointer, not a u64.
Please file an issue on the repository and include the code that triggered this error.

Before IR refactor:

19 |   s.update(10);
   |            ^^ Internal compiler error: Destination arg for load/store is not valid.
Please file an issue on the repository and include the code that triggered this error.
20 | }

So this doesn't look like a regression.

If I try to do the same with a regular function, instead of a method:

13 | fn test(mut data: u64) {
   |             ^^^^ This parameter was declared as mutable, which is not supported yet, did you mean to use ref mut?
14 |   data = 0
15 | }

So we maybe just need to apply the ref/mut parameter checking we do for impl functions too?

[mohammadfawaz]

Ah excellent. Yeah we just have to disallow mut without ref for method parameters everywhere