'Dropping temporaries' prevents borrow check to succeed in match arm without shared ownership

[Byron]

I tried this code (playground link):

struct Owner {
    to_be_shared: String,
}

impl Owner {
    fn share(&mut self) -> Result<Box<dyn ToString + '_>, ()> {
        Ok(Box::new(&self.to_be_shared) as Box<dyn ToString>)
    }
    fn do_something_else(&mut self) {}
}

fn use_owner(mut owner: Owner) {
    let _v = match owner.share() {
        Ok(v) => v,
        Err(()) => {
            owner.do_something_else();
            return;
        }
    };
}

fn main() {
    let owner = Owner {
        to_be_shared: String::new(),
    };
    use_owner(owner);
}

I expected to see this happen:

The code compiles. Especially it's possible to call owner.do_something_else() in the match branch dealing with the error, which does not borrow from owner.

Instead, this happened:

rror[E0499]: cannot borrow `owner` as mutable more than once at a time
  --> src/main.rs:16:13
   |
13 |     let _v = match owner.share() {
   |                    -------------
   |                    |
   |                    first mutable borrow occurs here
   |                    a temporary with access to the first borrow is created here ...
...
16 |             owner.do_something_else();
   |             ^^^^^ second mutable borrow occurs here
...
19 |     };
   |      - ... and the first borrow might be used here, when that temporary is dropped and runs the destructor for type `std::result::Result<std::boxed::Box<dyn std::string::ToString>, ()>`

According to The Rust Reference:Destructors I believe the drop scope should be relative to each match arm, not according to the outer scope. It appears that Rust still wants to drop the Result<_, _> which is already dissolved in the match arm.

Also, if no destructor is involved, the code compiles as expected.

The issue first occurred in gitoxide, and here is theworkaround.

Meta

rustc --version --verbose:

rustc 1.46.0 (04488afe3 2020-08-24)
binary: rustc
commit-hash: 04488afe34512aa4c33566eb16d8c912a3ae04f9
commit-date: 2020-08-24
host: x86_64-apple-darwin
release: 1.46.0
LLVM version: 10.0

rustc +nightly --version --verbose:

rustc 1.48.0-nightly (397b390cc 2020-08-27)
binary: rustc
commit-hash: 397b390cc76ba1d98f80b2a24a371f708dcc9169
commit-date: 2020-08-27
host: x86_64-apple-darwin
release: 1.48.0-nightly
LLVM version: 11.0

[csmoe]

As you might notice in the reference, the scrutinee of match isn't a temporary scope, temporary inside owner.share() will live until the end of match { ... }, so we have two mutable references here created: a) by share(); b) by do_something_else()

[Byron]

Thanks for taking the time. I suspected that this might actually be documented behaviour, but hope there is a shared understanding that this behaviour is surprising and undesirable. To the 'user' of the match statement, it does look like it's clear that in the match arm there is nothing with a constructor anymore, it seems like the Result has already been 'destructured'. Also there is no obvious way this can be achieved, as a workaround.

Do you see a way to improve the user experience around this issue? Maybe with a compiler error that suggests a workaround in these situations? Maybe there is even a way to change the behaviour of Rustc, very much in the same vein as what happened when NLL was introduced. Match statements suddenly became very intuitive to use.

Thanks again for your continued support.

[Byron]

I am now working around this issue by disabling the drop code of the box by leaking it temporarily. This being possible, to my mind, hints at the current handling of destructors in match being a technical shortcoming rather than a necessity.

For the example provided here in this issue, here is the corresponding workaround (playground):

struct Owner {
    to_be_shared: String,
}

impl Owner {
    fn share(&mut self) -> Result<Box<dyn ToString + '_>, ()> {
        Ok(Box::new(&self.to_be_shared) as Box<dyn ToString>)
    }
    fn do_something_else(&mut self) {}
}

fn use_owner(mut owner: Owner) {
    // disable drop code by leaking
    let _v = match owner.share().map(Box::leak) {
        Ok(v) => Ok::<_, ()>(v),
        Err(()) => {
            owner.do_something_else();
            return;
        }
    }
    .map(|leaked| unsafe { Box::from_raw(leaked as *mut _) }); // undo the leak right away, so the drop code runs after the match
}

fn main() {
    let owner = Owner {
        to_be_shared: String::new(),
    };
    use_owner(owner);
}

[csmoe]

Not sure the scrutinee temporary case can be improved, but the error message should be more informative at least, reopen for better suggestion.

[Byron]

Having received some pain from this issue, I should be able to come up with a message that would have helped me.
It's a tough one, as to me it made (and still makes) no sense to have the whole match expression as drop scope, which is quite clearly indicated by the message already.

A helpful suggestion would be one that helps working around the issue, and I don't know if the 'leak and unleak' workaround is anything we would want to suggest here due to the need for unsafe.

[SkiFire13]

This could be a variation of #47680, but it doesn't compile when using -ZPolonius.

Also note that this is related to the use of + '_. If share returned a Result<&'_ dyn ToString, ()> or a Result<Box<&'_ dyn ToString>, ()> it would work.

Another way to make it work would be to manually drop the value returned from share in the Err branch. This can be done by either saving the result in a temporary variable before the match or by using a @ binding:

fn use_owner(mut owner: Owner) {
    let o = owner.share();
    let _v = match o {
        Ok(v) => v,
        Err(()) => {
            drop(o);
            owner.do_something_else();
            return;
        }
    };
}

or

fn use_owner(mut owner: Owner) {
    let _v = match owner.share() {
        Ok(v) => v,
        e @ Err(()) => {
            drop(e);
            owner.do_something_else();
            return;
        }
    };
}