MCP: Allowing the compiler to eagerly drop values

[djc]

Proposal

Summary and problem statement

Allow the compiler to drop values (when they are "dead" in the control flow graph liveness sense) before the end of the lexical scope in order to make Rust more ergonomic to write.

Motivation, use-cases, and solution sketches

There are a number of cases where the compiler currently requires more elaborate code than it perhaps should in order to be able to clean things up in the correct order.

1. impl Drop invalidates code

While refactoring Quinn, I ran into this issue:

fn simple() {
    let mut foo = Foo;
    let mut adapter = foo.simple();
    adapter.next();
    adapter.next();
    let mut adapter = foo.simple();
    adapter.next();
}

struct FooAdapter<'a>(&'a mut Foo);

impl<'a> FooAdapter<'a> {
    fn next(&mut self) -> Option<usize> {
        Some(3)
    }
}

fn sophisticated() {
    let mut foo = Foo;
    let mut adapter = foo.sophisticated();
    adapter.next();
    adapter.next();
    let mut adapter = foo.sophisticated();
    adapter.next();
}

struct FooAdapterWithDrop<'a>(&'a mut Foo);

impl<'a> FooAdapterWithDrop<'a> {
    fn next(&mut self) -> Option<usize> {
        Some(5)
    }
}

impl<'a> Drop for FooAdapterWithDrop<'a> {
    fn drop(&mut self) {
        println!("droppy");
    }
}

struct Foo;

impl Foo {
    fn simple(&mut self) -> FooAdapter<'_> {
        FooAdapter(self)
    }
    
    fn sophisticated(&mut self) -> FooAdapterWithDrop<'_> {
        FooAdapterWithDrop(self)
    }
}

While simple() compiles, sophisticated() does not:

error[E0499]: cannot borrow `foo` as mutable more than once at a time
  --> src/lib.rs:24:23
   |
21 |     let mut adapter = foo.sophisticated();
   |                       --- first mutable borrow occurs here
...
24 |     let mut adapter = foo.sophisticated();
   |                       ^^^ second mutable borrow occurs here
25 |     adapter.next();
26 | }
   | - first borrow might be used here, when `adapter` is dropped and runs the `Drop` code for type `FooAdapterWithDrop`

I found it surprising that just implementing Drop for a type can cause downstream code to trivially fail to compile, for what I find to be no good reason: the first adapter is dead where the second one is instantiated, so I feel the compiler should allow this.

2. Dropping locks before await points

Here's a piece of code from bb8:

let (tx, rx) = oneshot::channel();
{
    let mut locked = self.inner.internals.lock();
    let approvals = locked.push_waiter(tx, &self.inner.statics);
    self.spawn_replenishing_approvals(approvals);
};

match timeout(self.inner.statics.connection_timeout, rx).await {
    Ok(Ok(mut guard)) => Ok(PooledConnection::new(self, guard.extract())),
    _ => Err(RunError::TimedOut),
}

I think the inner lexical scope there should not be necessary. The compiler should know to drop locked before the timeout().await.

Possible solutions

An ideal solution might be that the compiler can reason about "pressure" to drop eagerly and will do so automatically when needed. However, this is likely not viable without an edition change, because it would drop locks and other guard types more eagerly to the point where previously working code can become incorrect.

Failing that, a more compatible solution might be to have an opt-in EagerDrop marker trait that can be implemented by types that want to opt in to to having their destructors run more eagerly when viable.

Prioritization

I think this is aligned with the lang team priority to improve borrow checker expressiveness and other lifetimes issues. It might also indirectly help with async code insofar as await points can apply "pressure" to drop more eagerly, making the use of locks and other Drop implementers in async code easier.

Links and related work

I'm not aware of any particular related work, although I think lock guards in async code come up frequently in support forums.

Initial people involved

So far, I have informally discussed this with @nikomatsakis, who recommended I submit this project proposal.

What happens now?

This issue is part of the experimental MCP process described in RFC 2936. Once this issue is filed, a Zulip topic will be opened for discussion, and the lang-team will review open MCPs in its weekly triage meetings. You should receive feedback within a week or two.

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

[rustbot]

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

[scottmcm]

I found it surprising that just implementing Drop for a type can cause downstream code to trivially fail to compile

Note that this is true for other reasons as well. For example,

pub struct Foo { pub a: String, pub b: String }

//* Comment this and the code compiles
impl Drop for Foo {
    fn drop(&mut self) {}
}
//*/

pub fn demo(x: Foo) -> String {
    x.a
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=991039b157566767249ca1664e5dcfc6

[nikomatsakis]

So based on what I saw it seems like a lot of challenges were raised in Zulip. It'd be nice to get a summary of the discussion -- what concerns were raised, what are some interesting examples, and so forth. I think there may be room to do some smaller steps, such as adding lints that suggest explicit drops, or investigations into how frequently various code patterns occur in the wild.

[eholk]

Something like this might help reduce the cases where we capture too aggressively in generators (rust-lang/rust#69663).

[nikomatsakis]

True. Nonetheless, closing this for now. We captured the output from the discussion in the design note and there's a few too many challenges to tackle otherwise at this time.