Liveness, borrowck and last use interact badly

[catamorphism]

This arose from #2584. This code snippet from trans::base::trans_assign_op illustrates the problem:

        ret move_val(bcx, DROP_EXISTING, lhs_res.val,
                     // FIXME: should kind be owned?
                     {bcx: bcx, val: target, kind: owned},
                     dty);

bcx has type block, which is now a newtype-like enum whose LHS is an @ of a class. The bug is exposed by the fact that block doesn't get treated as immediate even though its representation is a box, but that's not the main bug. The main problem is that there's an implicit move because of the second use of bcx as a field in the record literal: it's a last use, implying that bcx gets zeroed at that point. Normally, borrowck would flag a move of something that has a reference to it (the reference gets brought about by the first instance of bcx, as the first argument to move_val). But since the implicit moves due to last-use information aren't visible to borrowck, the code happily compiles, runs, and then segfaults when move_val dereferences its first argument.

@nikomatsakis and I discussed this a bit on IRC and there's not a clear solution (though there is an obvious workaround for #2584). Perhaps the answer is to just remove last-use analysis.

[nikomatsakis]

Here is a targeted test case:

fn a_val(&&x: ~int, +y: ~int) -> int {
    *x + *y
}

fn main() {
    let z = ~22;
    a_val(z, z);
}

[nikomatsakis]

I see two possible solutions. One is to have borrowck remove the entries from last_use. This sounds both complex and a bit fragile. The other is to remove last_use and ask users to manually annotate moves.

It is a bit unclear to me what will be the impact of removing last_use on usability. On the one hand, it's a nice way to avoid excess use of the unary move operator. On the other hand, it's a common source of confusion ("why am I getting a copy warning here but not there? oh, I see, that's a last-use"). Also, when you add borrowck into the equation, it's one more thing that users have to think about ("it's not a last use because there is an existing alias in scope"). On the other hand, people will probably end up understanding the idea of "alias in scope" anyway.

[catamorphism]

A standalone test case for this is under run-pass/issue-2633.rs (currently xfailed).

[nikomatsakis]

Another example making use of by-value:

fn a_val(++x: @int, +y: @int) -> int {
    free(y);
    #debug["deref"];
    *x
}

fn free(-x: @int) {}

fn main() {
    let z = @22;
    assert 22 == a_val(z, z);
}

[pcwalton]

I'm in favor of getting rid of the last use analysis. Programmers should not have to run a liveness pass in their heads to figure out where the moves are.

A possibility here is to re-purpose the last use analysis into a fixit for the no-implicit-copies warning. That is, you might get warnings like so if liveness detects that the variable in question is dead:

foo.rs:20: warning: copying a value of type '~str' requires memory allocation
foo.rs:20: note: suggest use of 'move' to avoid the copy

[catamorphism]

I like the idea of using last-use as a source of data for improving error messages, but not for guiding code generation at all.

[nikomatsakis]

I think I like @pcwalton's idea as well. However, that seems like a decision that should be brought up at the Tuesday meeting. For the very short term, it's probably easy enough to have borrowck remove unsafe entries from the last_use table. I'm not thrilled about the idea because we tend to make tables that are built-up by a pass and then read-only thereafter, which seems like reasonable practice.

[catamorphism]

At the meeting today, we agreed to adopt @pcwalton 's idea to make last-use solely a hint source for error messages.