Conditionally moving a reference in a loop results in a compiler crash

[yorickpeterse]

Please describe the bug

The following code crashes the compiler:

class async Main {
  fn async main {
    let a = [10]
    let b = mut a

    while true {
      let el = if true { mut a } else { b }
    }
  }
}

The crash is as follows:

thread 'main' panicked at compiler/src/mir/passes.rs:3940:69:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The crash appears to happen due to the compiler not finding a drop register where it expects one, or by perhaps trying to drop a reference as an owned value (which wouldn't have a drop register).

Please list the exact steps necessary to reproduce the bug

Save the following as test.inko:

class async Main {
  fn async main {
    let a = [10]
    let b = mut a

    while true {
      let el = if true { mut a } else { b }
    }
  }
}

Then run inko check test.inko.

Operating system

Arch Linux

Inko version

main

Rust version

1.74.1

[yorickpeterse]

A simpler way to reproduce this problem:

class async Main {
  fn async main {
    let a = [10]
    let b = mut a

    {
      if true { mut a } else { b }
    }
  }
}

The issue is that in LowerMethod::body(), the last expression in a body/scope is processed using LowerMethod::output_expression(). If the value returned happens to be a reference, we use the register as-is and mark it as moved. What we should be doing here is creating and returning a new reference to b.

This does pose an interesting problem: if the last expression is something that creates a new reference (e.g. ref bla), we don't want to create another reference just so we can return it. But if a local variable that is typed as a reference is returned, we should create a new reference to it. At the same time we shouldn't create a reference just because we refer to a variable, i.e. b.foo() shouldn't result in a new reference being created for b, as that's just redundant.

[yorickpeterse]

I'm going to give this a night's thought, but I think we can do the following:

Local variables should use RegisterKind::Variable. If LowerMethod::output_expression() encounters such a register and it's typed as a reference, we create a new register containing a new reference to the variable and return that. This is similar to how we handle returning self or fields.

This can however result in redundant reference counts, such as the following:

fn foo(a: ref Whatever) -> ref Whatever {
  return a
}

Here the above rule would result in us creating a new ref Whatever and returning that, instead of returning the existing reference as-is.

Perhaps we should distinguish between an output expression for return/throw, and an output expression for an implicit scope return, rather than trying to shoehorn all of this into the same method.