Fix reassignment of struct members in impl self

[mohammadfawaz]

Closes: #1169
When looking for the value to reassign, also look for it in the argument list of the function.

[mohammadfawaz]

@otrho this looks a bit weird to me even though it's functional. I understand that this IR is before the optimizer runs, but the fact that we're inserting into a const seems odd. Is this expected?

[otrho]

Yep, conceptually I was thinking functional style, so insert_value isn't modifying the original, but creating a new value. v2 is not a modified v0, it's a whole new value. But this will go away with the struct refactor and everything goes on the stack.

[mohammadfawaz]

Right, yeah that makes sense. This is SSA anyways.. I don't know what I was thinking 😂

[mohammadfawaz]

I think these changes are due to the fact that we're now creating get_ptr before compiling the RHS.

[otrho]

OK, v4 is used way down below for the store in block1, where the old get_ptr was, which is a bit unfortunate.

[otrho]

I've come across a slightly different scenario where the AST is saying that the following:

fn f(a: u64) {
    let a = a + 1;
    ...
}

Is a re-assignment, where IMO this is a new declaration which is shadowing the function argument. I didn't raise an issue at the time though...

This change is necessary, because it (e.g. self.a = 0 from the test) is a genuine re-assignment, but it'll also hide the above issue. But maybe the above issue isn't an issue.

[otrho]

OK, v4 is used way down below for the store in block1, where the old get_ptr was, which is a bit unfortunate.

[mohammadfawaz]

I've come across a slightly different scenario where the AST is saying that the following:

fn f(a: u64) {
    let a = a + 1;
    ...
}

Is a re-assignment, where IMO this is a new declaration which is shadowing the function argument. I didn't raise an issue at the time though...

This change is necessary, because it (e.g. self.a = 0 from the test) is a genuine re-assignment, but it'll also hide the above issue. But maybe the above issue isn't an issue.

You're right.. a new variable is being declared via let so this is certainly not a reassignment.

[mohammadfawaz]

I've come across a slightly different scenario where the AST is saying that the following:

fn f(a: u64) {
    let a = a + 1;
    ...
}

Is a re-assignment, where IMO this is a new declaration which is shadowing the function argument. I didn't raise an issue at the time though...
This change is necessary, because it (e.g. self.a = 0 from the test) is a genuine re-assignment, but it'll also hide the above issue. But maybe the above issue isn't an issue.

You're right.. a new variable is being declared via let so this is certainly not a reassignment.

@otrho I looked at the AST of the following code:

fn foo(a: u64) {
    let a = a + 1;
}

fn main() {
    foo(5);
}

and I don't see a Reassignment in the AST 🤔 , and the IR has a new local ptr with an _ as expected:

local ptr u64 a_

[otrho]

Argh. It was something like that..! I think it was in the enum_if_let_large_types test. Doesn't matter for now, if it becomes and issue we can deal with it then, there's plenty of other stuff to do for now. 🙂