Borrow check needs to consider moves into patterns

[bblum]

struct Dummy {
    x: int
}

impl Dummy {
    fn new(x: int) -> Dummy {
        Dummy { x: x }
    }
}

impl Drop for Dummy {
    fn drop(&self) {
        error!("Destructor %d", self.x);
    }
}

fn welp<T>(a: ~T) -> T {
    let ~x = a;
    x
}

fn main() {
    let o = ~Dummy::new(5);
    welp(o);
}

(edit: a similar problem appears for let a@b = c; see comment below.)

[eholk]

I wonder if this is related to #2548...

[nikomatsakis]

Interesting. There may be two bugs here. The first, I would think, is that this let assignment should not be legal. The liveness code would not ordinarily allow you to move from the inside of managed data.

The second is the dtor running twice. This program exhibits the same behavior:

class dummy { let x: int; new(x: int) { self.x = x; } drop { #error["Destructor %d", self.x]; } }

fn welp<T>(+a: ~T) -> T { 
    let ~x <- a;
    x   
}

fn main() {
    let o = ~dummy(5);
    welp(o);
}   

My guess is that the running twice part is a dup of #2548 or at least related. The other part is a shortcoming of liveness/borrowck... I'm not precisely sure where the check ought to be happening that prevents moving from the interior of a managed box, but it ought to happen somewhere. Probably borrowck since it already has the code to match up patterns and so forth.

[nikomatsakis]

actually thinking more I think it's borrowck's responsibility to prevent moves from illegal places.

[bblum]

Thinking more about this, I realised you should never be able to get a noncopyable out of an @-box once it's in - it's a similar problem as option::unwrap except @-boxes always "implicitly" have a loan out on their contents.

Probably moving out of an @-box should always be illegal, and you should have to do it explicitly by copy *a instead. When let-ref appears, deconstructing by-ref should be legal again.

It's interesting that the destructor runs twice in your example - that one seems more analogous to option::unwrap; seems like that should be allowed.

Also interestingly, since let is by-copy currently, this also breaks (and is nonsense, when you think about it): let @x = a

[nikomatsakis]

I updated the title of this bug to reflect the problem that the move into @P is permitted at all. The other bug (that the dtor runs twice) is I think a dup, as I said before.

[bblum]

if match is going to be copy by default, I can see the same problem trying to bind the inside of an @ in a match.

[nikomatsakis]

preventing copies is really the job of the kind checker, but yeah we'll have to be sure and get this right. I'm sure we don't now, should open a bug on it.

[bblum]

let a@b = c also creates a copy of c when it shouldn't. The fix for this should be the same as for that, except in the pat_ident case instead of the pat_box case.

[bblum]

I suspect this bug exists with move bindings in match arms too. I think this bug should instead be titled "Patterns should never be able to move out of @-boxes".

[thestinger]

@nikomatsakis: is this still a problem?

[graydon]

reproduced on 2013-03-22

[graydon]

non-critical for 0.6, de-milestoning

[catamorphism]

Nominating for milestone 2, backward-compatible

[nikomatsakis]

An update: The general approach for preventing moves in patterns has shifted a bit. In particular, we are currently detecting such moves in check_match. I am not 100% sure if this is a good idea, because it's not especially DRY, but there is a certain logic to it. In general though I believe there are problems with let patterns not being checked very thoroughly.

[huonw]

I added a test in #6385 which I had to xfail because of this.

[graydon]

accepted for production-ready milestone

[graydon]

Triage: reproduced 2013-07-25, editing code to compensate for language drift.

[nikomatsakis]

@graydon I believe this bug was fixed.

In particular:

1. @huonw's test now reports the expected error (albeit with a slightly different error message).
2. This test only prints "Hi" once:

struct dummy {
    x: int
}

impl Drop for dummy {
    fn drop(&self) {
        println(fmt!("Hi: %d", self.x));
    }
}

fn welp<T>(a: ~T) -> T {
    let ~x = a;
    x
}

fn main() {
    let o = ~dummy { x: 5 };
    welp(o);
}

1. The test in the initial comment (which looks to have been updated?) only prints the destructor once.

Which test did you run? Presumably the test in the initial comment?

There are still actions required:

1. Delete @huonw's test, which is superceded by compile-fail/borrowck-move-out-of-struct-with-dtor.rs
2. Perhaps add the example as a test? Unfortunately, the tests in run-pass for moves are given non-helpful names like move-1 and move-2 so it's hard to tell if a test exists for this condition or not.

[pcwalton]

Closing because this bug has long since been fixed.