Fix guard variable scoping

[EliasDerHai]

Fixes guard expression with variable shadowing brakes type guarantee

This is my first contribution. Please let me know if I messed up.

I updated 2 snapshots, but don't know if this is fine.

Regarding the change in the decision.rs:

            let check_bindings = this.variables.bindings_ref_doc(&check_bindings);

is done in the main scope of the function but through bindings_ref_doc -> body_binding_doc -> next_local_var the expression_generator's current_scope_vars is mutated - which is reused in the other branches. This leads to the issue that I observed.

I tried to cover the fix with a minimal test.

[lpil]

Thank you! I left a question on the test below

[lpil]

Isn't this an error in JavaScript? There is already a first defined in this function

[EliasDerHai]

No it's not an error in js since let and const are block-scoped.

Shadowing doesn't pollute the outer scope - in contrary to var which is bad practice exactly for that reason.

I ran a quick test on how my change behaves for "double-guards" - eg. :

pub fn main() {
  let x = 42
  case x {
    x if x < 0 -> x
    x if x > 40 -> x * 2
    _ -> 0
  }
}

This previously compiled to:

export function main() {
  let x = 42;
  let x$1 = x;
  if (x$1 < 0) {
    return x;
  } else {
    let x$2 = x;
    if (x$2 > 40) {
      return x$2 * 2;
    } else {
      return 0;
    }
  }
}

with my change compiles to:

export function main() {
  let x = 42;
  let x$1 = x;
  if (x$1 < 0) {
    return x;
  } else {
    let x$1 = x;
    if (x$1 > 40) {
      return x$1 * 2;
    } else {
      return 0;
    }
  }
}

Again we see that x$1 is repeatedly defined (in different scopes). - Fine by js but maybe odd for humans to read. It could very well be though that what we declare as x in gleam are different types. Back-tracing declarations from upper scopes for the purpose of potential reuse doesn't make much sense and is probably quite error-prone though.

Each guard produces a declaration as far as I can see. Since expressions within if-conditions aren't possible we're forced to declare in upper scope. if we shadow vars or 'soft-shadow' with counter increment doesn't matter much or?

Do you suggest putting more effort to avoid variable shadowing in js?
Do you see a way how the change could cause a bug?

[lpil]

Oh yes! Thank you for your thorough explanation. Very silly of me there

[lpil]

Thank you again!! Very nice

[lpil]

Oh yes! Thank you for your thorough explanation. Very silly of me there