Always separately bind RAII guards: Use scope instead of explicit drop.

[vks]

I think

fn use_mutex(m: sync::mutex::Mutex<int>) {
    {
        let guard = m.lock();
        do_work(guard);
    }
    // do other work
}

is better than

fn use_mutex(m: sync::mutex::Mutex<int>) {
    let guard = m.lock();
    do_work(guard);
    drop(guard); // unlock the lock
    // do other work
}

because it uses the scope instead of explicitly calling the destructor.

[sfackler]

Why is that better? That argument seems a bit circular.

[vks]

Let me try to expand my argument:

If you use the scope rather than calling the destructor manually, it is impossible to reuse the object after destroying it (it would give a compile time error about an undefined variable).

(I don't know whether Rust is clever enough to give a compile time error if you try to use an object after droping it. If this is the case, it makes less of a difference.)

Also, I think it makes the lifetime of the object easier to parse.

[sfackler]

You cannot use a variable after it has been moved. If you could, it would be a massive soundness hole:

struct Foo {
    a: int
}

impl Drop for Foo {
    fn drop(&mut self) {}
}

fn main() {
    let foo = Foo { a: 10 };
    drop(foo);
    println!("{}", foo.a);
}

test.rs:12:20: 12:25 error: use of partially moved value: `foo.a`
test.rs:12     println!("{}", foo.a);
                              ^~~~~
note: in expansion of format_args!
<std macros>:2:23: 2:77 note: expansion site
<std macros>:1:1: 3:2 note: in expansion of println!
test.rs:12:5: 12:27 note: expansion site
test.rs:11:10: 11:13 note: `foo` moved here because it has type `Foo`, which is non-copyable (perhaps you meant to use clone()?)
test.rs:11     drop(foo);
                    ^~~
error: aborting due to previous error

[vks]

Interesting, so the compiler catches this. I guess the only advantage of having a new scope is then that the lifetime is more obvious. (I also think it looks cleaner, but that is probably bikeshedding.)

[aturon]

Thanks @vks and @sfackler for the discussion. It seems like we could probably use a guideline for whether to prefer explicit nested scopes or drop when you want to destroy an object early. I suspect that nested scopes is actually the prevailing style, but I'll check.

[sfackler]

Block-scoping may be the way to go to be more consistent with the way that borrows work, at least until rust-lang/rust#6393 is fixed.

[pnkfelix]

I do think that explicit nested scopes are the prevailing style. However, once the change to our drop semantics changes to require all control flow joins to have the same set of dropped things (see this comment), then explicit calls to drop are going to might be necessary anyway.

[ghost]

I'm not sure whether this discussion is still relevant with current Rust (1.6), but sometimes you don't really know whether you own the lock or have a borrow to it.

Using drop explicitly is easier than figuring out the type of a variable.

[pnkfelix]

once the change to our drop semantics changes to require all control flow joins to have the same set of dropped things [...]

N.B. this is no longer the plan of record., so thst argument is moot

[pnkfelix]

@pijul if all you have is a borrow of a lock, then dropping it (the borrow) will have no effect. So I am not sure what distinction you were drawing there.

[ghost]

Ok, probably my misunderstanding then. Sorry for the noise.