Uniformly prevent values with destructors from being disassembled/moved out of

[nikomatsakis]

We need to put in place three checks (at least?) to prevent values with destructors from being moved out of:

• No pattern with by-move bindings that matches a type with a dtor (I think this is implemented)
• No moving from a subcomponent of a type with a dtor (e.g., x.f where x has a dtor). Right now the borrow checker doesn't allow such moves, but there is no deep reason why it shouldn't, and the rest of the code is kind of setup to allow them whenever borrow checker gives the nod.
• No moving out of a value with a dtor in FSU: S { a, b, ..c } where c has a dtor, even if this would otherwise be legal

[msullivan]

Has any of this been done?

[nikomatsakis]

I actually this this is done. Let me double-check the tests.

[nikomatsakis]

See e.g. compile-fail/borrowck-move-out-of-struct-with-dtor.rs

[nikomatsakis]

Not sure about the FSU case, and the test doesn't cover the let y = x.f

[nikomatsakis]

It occurs to me that this test is only present on the branch for PR #7262

[thestinger]

@nikomatsakis: can this be closed now?

[pnkfelix]

@thestinger sounds like it needs tests. I will tag as such (and also poke around to check what the situation is with respect to the tests).

[pnkfelix]

Or rather, I think the first two bullets already have tests:

• compile-fail/borrowck-move-out-of-struct-with-dtor.rs
• compile-fail/borrowck-move-out-of-tuple-struct-with-dtor.rs

Now I'm just trying to understand what Niko was getting at about the functional struct update (FSU) case...

[pnkfelix]

Okay, the borrow checker currently does not prevent someone from doing this, but I think @nikomatsakis wants it to be prevented:

use NC = std::util::NonCopyable;

#[cfg(version2)]
struct S { a: int, b: int, p: ~str, nc: NC }
#[cfg(version2)]
fn make_s(a: int) -> S { S{ a: a, b: 0, p: ~"A", nc: NC } }
#[cfg(version2)]
fn ownmut(s: &mut S) { s.b = 1; s.p.push_char('B'); }

#[cfg(not(version2))]
struct S { a: int, b: int, nc: NC }
#[cfg(not(version2))]
fn make_s(a: int) -> S { S{ a: a, b: 0, nc: NC } }
#[cfg(not(version2))]
fn ownmut(s: &mut S) { s.b = 1; }

impl ToStr for S {
    #[cfg(version2)]
    fn to_str(&self) -> ~str {
        "S { a: "+ self.a.to_str() + 
            ", b: "+ self.b.to_str() +
            ", p: "+ self.p + " (nc) }"
    }
    #[cfg(not(version2))]
    fn to_str(&self) -> ~str {
        "S { a: "+ self.a.to_str() +
            ", b: "+ self.b.to_str() + " (nc) }"
    }
}

impl Drop for S {
    fn drop(&mut self) {
        println!("{} is dropped", self.to_str());
    }
}

fn main() {
    let s0 = make_s(0);
    println!("s0: {}", s0.to_str());

    // Shouldn't be allowed: A move out of value with a dtor via
    // Functional Struct Update (FSU):
    let mut s2 = S{a: 2, ..s0};

    // Uncommenting below will fail to compile, b/c s0 has been moved
    // println!("s0: {}", s0.to_str());

    ownmut(&mut s2);

    println!("s2: {}", s2.to_str());
}

There seems like there's good reason for preventing this, at least in our current system, because its not clear how you should go about destructing instances of things like S where some of their parts have been reused (i.e. moved) via functional struct update.

I put two versions of the code above. In one version, S carries trivial payloads, and the compiled program "just" makes two destructor invocations at the end of main: one for s2, and another for s0. In "version2", I added a ~str payload to it (but left the struct-update alone, so the reference gets copied from s0 to s2).

Here's the results:

% rustc /tmp/f.rs && /tmp/f
warning: no debug symbols in executable (-arch x86_64)
s0: S { a: 0, b: 0 (nc) }
s2: S { a: 2, b: 1 (nc) }
S { a: 2, b: 1 (nc) } is dropped
S { a: 0, b: 0 (nc) } is dropped
% rustc --cfg version2 /tmp/f.rs && /tmp/f
warning: no debug symbols in executable (-arch x86_64)
s0: S { a: 0, b: 0, p: A (nc) }
s2: S { a: 2, b: 1, p: AB (nc) }
S { a: 2, b: 1, p: AB (nc) } is dropped
Segmentation fault: 11
% 

So, yeah, this is not yet done, and its not just a matter of E-needstest; I'll take that flag back off.