LensPlaysGames/LensorCompilerCollection

Potentially incorrect codegen of variable reassignment

Sirraide opened this issue · 3 comments

This is in regard to the following check
https://github.com/LensPlaysGames/FUNCompiler/blob/061ede20453730e12b935584d52713ebbb34b622/src/codegen.c#L127-L129

We typically don’t want to emit the same expr twice what with SSA and all that. However, there is an exception: If a variable declaration or the same variable reference is used twice, we need to emit the load twice because the variable’s value may have changed inbetween.

I’m not sure if this really is an issue, but we should probably add a test for that (e.g. make a test that reassigns a variable a couple of times and see if it’s correct). This may not be a problem in our implementation (because we create a new varref node every time the variable name is used iirc), but it’s just something that came to mind and that I wanted to mention just in case because I feel like it might warrant at least some investigation.

the same variable reference is used twice

Note that this is about using the same variable reference node multiple times. Multiple variable reference nodes to the same variable (which I think is what we’re doing atm) should work just fine.

This is going to have even greater importance now that macro arguments with the expr_once selector will have this exact property of a single var ref node being referenced multiple times.

I think one way of solving this would be to add an ephemeral flag to each AST node that indicates whether the node should be re-emitted every time it’s used.