Working around overzealous drop check

[cole-miller]

This playground shows the shape of the code I'm writing. It compiles fine when the _nontrivial_drop field of Encoder is commented out. However, when that field is added, we get some errors from the drop check (I think):

error[E0499]: cannot borrow `*stuff` as mutable more than once at a time
  --> src/lib.rs:58:57
   |
51 |         stuff,
   |         ----- first mutable borrow occurs here
...
56 |             encoder = match encoder.block() {
   |             ------- first borrow might be used here, when `encoder` is dropped and runs the destructor for type `Encoder<'_>`
57 |                 Either::Left(enc) => break enc,
58 |                 Either::Right(save) => save.resume(grow(stuff)),
   |                                                         ^^^^^ second mutable borrow occurs here

error[E0499]: cannot borrow `*stuff` as mutable more than once at a time
  --> src/lib.rs:64:57
   |
51 |         stuff,
   |         ----- first mutable borrow occurs here
...
61 |         encoder = loop {
   |         ------- first borrow might be used here, when `encoder` is dropped and runs the destructor for type `Encoder<'_>`
...
64 |                 Either::Right(save) => save.resume(grow(stuff)),
   |                                                         ^^^^^ second mutable borrow occurs here

For more information about this error, try `rustc --explain E0499`.

Is there actually a soundness issue here? I wouldn't think so, since the drop glue for Encoder<'a> won't touch the &'a mut [u8] field. And if my code is sound, how can I (safely or unsafely) work around the drop check in this case? (I don't think the eyepatch helps.)

[jonh]

You write functions to return the item you intend to still use. e.g.
fn block(self) -> Either<BlockEncoder<'a>, (EncoderSave, &'a mut Vec<u8>)> {

[steffahn]

I’ve done some reduction work to get a smaller example; here’s what I got so far:

#![allow(dead_code)]

enum Either<L, R> {
    Left(L),
    Right(R),
}

struct Encoder<'a, T> {
    stuff: &'a mut [u8],
    _nontrivial_drop: T,
}

struct EncoderSave<T>(T);

impl<T> EncoderSave<T> {
    fn resume<'a>(self, _stuff: &'a mut [u8]) -> Encoder<'a, T> {
        unimplemented!()
    }
}

impl<'a, T> Encoder<'a, T> {
    fn block(self) -> Either<(), EncoderSave<T>> {
        unimplemented!()
    }
}

fn grow(_: &mut Vec<u8>) -> &mut [u8] {
    unimplemented!()
}

// fails without the `: Copy`

// but then, works again, if the out-of-loop `let mut encoder`
// becomes an ordinary `let encoder =` inside of the loop
fn branch1<T: Copy>(mut save: EncoderSave<T>, stuff: &mut Vec<u8>) -> () {
    let mut encoder;
    loop {
        encoder = save.resume(grow(stuff));
        save = match encoder.block() {
            Either::Left(enc) => break,
            Either::Right(save) => save,
        };
    };
}

[quinedot]

One workaround is to create loop-local Encoders.

[quinedot]

Maybe this?
https://github.com/rust-lang/rust/issues/70919

[steffahn]

It’s not pretty or particularly DRY, but this compiles even though the control flow is essentially “the same”; probably because it successfully avoids mutating encoder or block_encoder in any loop that also involves repeated .resume(grow(stuff)) calls:

#![allow(dead_code)]

enum Either<L, R> {
    Left(L),
    Right(R),
}

struct Encoder<'a> {
    stuff: &'a mut [u8],
    _nontrivial_drop: Vec<u8>,
}

struct EncoderSave;

impl EncoderSave {
    fn resume(self, _stuff: &mut [u8]) -> Encoder<'_> {
        unimplemented!()
    }
}

impl<'a> Encoder<'a> {
    fn block(self) -> Either<BlockEncoder<'a>, EncoderSave> {
        unimplemented!()
    }
}

struct BlockEncoder<'a> {
    stuff: &'a mut [u8],
    _nontrivial_drop: Vec<u8>,
}

struct BlockEncoderSave;

impl BlockEncoderSave {
    fn resume(self, _stuff: &mut [u8]) -> BlockEncoder<'_> {
        unimplemented!()
    }
}

impl<'a> BlockEncoder<'a> {
    fn encode(self) -> Either<Encoder<'a>, BlockEncoderSave> {
        unimplemented!()
    }
}

fn grow(_: &mut Vec<u8>) -> &mut [u8] {
    unimplemented!()
}

fn go(stuff: &mut Vec<u8>) {
    let mut encoder = Encoder {
        stuff,
        _nontrivial_drop: Vec::new(),
    };
    let mut block_encoder;
    loop {
        match encoder.block() {
            Either::Left(enc) => {
                block_encoder = enc;
            }
            Either::Right(mut save) => {
                let mut bsave;
                loop {
                    'x: loop {
                        let mut encoder = save.resume(grow(stuff));
                        let mut block_encoder;
                        loop {
                            match encoder.block() {
                                Either::Left(enc) => {
                                    block_encoder = enc;
                                }
                                Either::Right(save_) => {
                                    save = save_;
                                    continue 'x;
                                }
                            }
                            match block_encoder.encode() {
                                Either::Left(enc) => {
                                    encoder = enc;
                                }
                                Either::Right(bsave_) => {
                                    bsave = bsave_;
                                    break 'x;
                                }
                            }
                        }
                    }
                    'y: loop {
                        let mut encoder;
                        let mut block_encoder = bsave.resume(grow(stuff));
                        loop {
                            match block_encoder.encode() {
                                Either::Left(enc) => {
                                    encoder = enc;
                                }
                                Either::Right(bsave_) => {
                                    bsave = bsave_;
                                    continue 'y;
                                }
                            }
                            match encoder.block() {
                                Either::Left(enc) => {
                                    block_encoder = enc;
                                }
                                Either::Right(save_) => {
                                    save = save_;
                                    break 'y;
                                }
                            }
                        }
                    }
                }
            }
        }
        match block_encoder.encode() {
            Either::Left(enc) => {
                encoder = enc;
            }
            Either::Right(mut bsave) => {
                let mut save;
                loop {
                    'y: loop {
                        let mut encoder;
                        let mut block_encoder = bsave.resume(grow(stuff));
                        loop {
                            match block_encoder.encode() {
                                Either::Left(enc) => {
                                    encoder = enc;
                                }
                                Either::Right(bsave_) => {
                                    bsave = bsave_;
                                    continue 'y;
                                }
                            }
                            match encoder.block() {
                                Either::Left(enc) => {
                                    block_encoder = enc;
                                }
                                Either::Right(save_) => {
                                    save = save_;
                                    break 'y;
                                }
                            }
                        }
                    }
                    'x: loop {
                        let mut encoder = save.resume(grow(stuff));
                        let mut block_encoder;
                        loop {
                            match encoder.block() {
                                Either::Left(enc) => {
                                    block_encoder = enc;
                                }
                                Either::Right(save_) => {
                                    save = save_;
                                    continue 'x;
                                }
                            }
                            match block_encoder.encode() {
                                Either::Left(enc) => {
                                    encoder = enc;
                                }
                                Either::Right(bsave_) => {
                                    bsave = bsave_;
                                    break 'x;
                                }
                            }
                        }
                    }
                }
            }
        }
    }
}

[steffahn]

In terms of practical workarounds: this seems to work nicely and without overhead:

fn go(stuff: &mut Vec<u8>) {
    let mut encoder = ManuallyDrop::new(Encoder {
        stuff,
        _nontrivial_drop: Vec::new(),
    });
    loop {
        let mut block_encoder = loop {
            encoder = ManuallyDrop::new(match ManuallyDrop::into_inner(encoder).block() {
                Either::Left(enc) => break enc,
                Either::Right(save) => save.resume(grow(stuff)),
            });
        };
        encoder = loop {
            block_encoder = match block_encoder.encode() {
                Either::Left(enc) => break ManuallyDrop::new(enc),
                Either::Right(save) => save.resume(grow(stuff)),
            };
        };
    }
}

Might need block_encoder wrapped similarly if that one has nontrivial drop, too.

---

There isn’t even any downsides such as leak on panic, since the encoder is always immediately consumed with …::into_inner without anything that can panic in-between. I think. It's probably easy to restructure the code in order to make this even more obvious.

[cole-miller]

(Looking over my code again I'm struck by the bad/confusing type and variable names. Sorry about that.)

@jonh

You write functions to return the item you intend to still use. e.g.
fn block(self) -> Either<BlockEncoder<'a>, (EncoderSave, &'a mut Vec<u8>)> {

Thanks, this seems promising. I'll report back whether I'm able to apply it to my real code.

@quinedot

One workaround is to create loop-local Encoders.

I need to study this a bit more before I can wrap my head around it. It seems like the key is to temporarily split the nontrivial-Drop part of Encoder from the &mut part, so the compiler can't see problematic interaction between them.

@quinedot

Maybe this? [snip]

Looks familiar. I guess I misidentified the issue in my OP—the destructor of Encoder<'a> never runs at all, because the value is consumed by Encoder::block instead, but the compiler doesn't see that for whatever reason.

@steffahn

In terms of practical workarounds: this seems to work nicely and without overhead: [snip]

Neat! I'd love to see whether the core logic here can be packaged up into a utility method on Encoder or something, because ultimately I want consumers of my library to be able to write this kind of loop in their own code.