Borrow checker problem when going generics

help
1

Hi folks,

I'm having a problem with some code that does work with a single type but breaks when trying to make it accept two very similar types using generics...

error[E0502]: cannot borrow `*buf` as mutable because it is also borrowed as immutable
  --> src/lib.rs:30:21
   |
22 |     fn wait_next<'a, T>(&'a mut self, stream: &mut TcpStream) -> Result<Option<T>, std::io::Error>
   |                  -- lifetime `'a` defined here
...
28 |             match buf[..*n].try_into() {
   |                   --------------------
   |                   |
   |                   immutable borrow occurs here
   |                   argument requires that `*buf` is borrowed for `'a`
29 |                 Ok(InputGeneric(data, used)) => {
30 |                     buf.copy_within(used..*n, 0);
   |                     ^^^ mutable borrow occurs here

error[E0502]: cannot borrow `*buf` as mutable because it is also borrowed as immutable
  --> src/lib.rs:35:46
   |
22 |     fn wait_next<'a, T>(&'a mut self, stream: &mut TcpStream) -> Result<Option<T>, std::io::Error>
   |                  -- lifetime `'a` defined here
...
28 |             match buf[..*n].try_into() {
   |                   --------------------
   |                   |
   |                   immutable borrow occurs here
   |                   argument requires that `*buf` is borrowed for `'a`
...
35 |                     let x = stream.read(&mut buf[*n..])?; // read a bit more data.
   |                                              ^^^ mutable borrow occurs here

For more information about this error, try `rustc --explain E0502`.
error: could not compile `playground` (lib) due to 2 previous errors

The generic code. I think I need a 'static bound somewhere saying that the returned value does not borrow from the buf, but I've tried everywhere and couldn't make it work:

    fn wait_next<'a, T>(&'a mut self, stream: &mut TcpStream) -> Result<Option<T>, std::io::Error>
    where
        InputGeneric<T>: TryFrom<&'a [u8]>,
    {
        let DataStream { buf, n, .. } = self;
        loop {
            match buf[..*n].try_into() {
                Ok(InputGeneric(data, used)) => {
                    buf.copy_within(used..*n, 0);
                    *n -= used;
                    return Ok(Some(data));
                }
                Err(e) => {
                    let x = stream.read(&mut buf[*n..])?; // read a bit more data.
                    if x == 0 {
                        return Ok(None);
                    }
                    *n += x;
                }
            }
        }
    }

The non-generic code that works. Note they are identical except for the type within the Ok:

    fn wait_next(&mut self, stream: &mut TcpStream) -> Result<Option<Data>, std::io::Error> {
        let DataStream { buf, n, .. } = self;
        loop {
            match buf[..*n].try_into() {
                Ok(Input(data, used)) => {
                    buf.copy_within(used..*n, 0);
                    *n -= used;
                    return Ok(Some(data));
                }
                Err(e) => {
                    let x = stream.read(&mut buf[*n..])?; // read a bit more data.
                    if x == 0 {
                        return Ok(None);
                    }
                    *n += x;
                }
            }
        }
    }

Here is a playground to simulate it. If you comment/uncomment both wait_next() methods, you can see that the non-generic works and the generic doesn't.

Could you please help me? What am I missing?

2

First of all, this bound is far too strict:

This says that you can convert an InputGeneric<T> from a reference with exactly the lifetime 'a and no other, but the lifetime 'a was used in &'a mut self, so that lifetime lasts for the entire function call, but you're trying to mutate buf in a loop, so borrows of buf cannot be valid for all of 'a.

The usual fix to this type of problem is to use a HRTB to say that any lifetime will do:

InputGeneric<T>: for<'b> TryFrom<&'b [u8]>,

However, that doesn't seem to work here, and I'm not sure why; possibly the compiler isn't smart enough to see the borrow can't outlast the try_from() call, or perhaps the mandatory invariance of trait lifetime parameters is getting in the way somehow. (Explained; see @drewtato's post below.) But there is another solution: define a trait which, like std::str::FromStr, puts the lifetime on the function rather than the trait:

trait FromBytes: Sized {
    type Error;
    fn from_bytes(buf: &[u8]) -> Result<Self, Self::Error>;
}

impl FromBytes for InputGeneric<Data> {
    ...

Replace your implementations and uses of TryFrom<&[u8]> with FromBytes, and the code will compile.

General lesson to take from this: choose, or design, traits to suit the borrowing pattern you have. Try to avoid lifetime parameters in trait bounds, when feasible — which is more often than you might think.

3 Likes
3

It's incredibly ugly but this works and should be correct:

fn wait_next<T>(&mut self, stream: &mut TcpStream) -> Result<Option<T>, std::io::Error>
where
    InputGeneric<T>: for<'a> TryFrom<&'a [u8]>,
    for<'a> <InputGeneric<T> as TryFrom<&'a [u8]>>::Error: 'static,
{
    let DataStream { buf, n, .. } = self;
    loop {
        if let Ok(InputGeneric(data, used)) = buf[..*n].try_into() {
            buf.copy_within(used..*n, 0);
            *n -= used;
            return Ok(Some(data));
        }
        let x = stream.read(&mut buf[*n..])?; // read a bit more data.
        if x == 0 {
            return Ok(None);
        }
        *n += x;
    }
}

You need to know that TryFrom::Error doesn't contain a reference to buf.

I also refactored your match into if-let, but I don't think that matters for this. Just a little cleaner. If you want to use the error, the match should work.

4 Likes
4

Here it is without the Error bound (which should be unnecessary) but with checking the result twice. This should get optimized into one check. If you need the guarantee, you can unwrap_unchecked.

fn wait_next<T>(&mut self, stream: &mut TcpStream) -> Result<Option<T>, std::io::Error>
where
    InputGeneric<T>: for<'a> TryFrom<&'a [u8]>,
{
    let DataStream { buf, n, .. } = self;
    loop {
        {
            let res = buf[..*n].try_into().ok();
            if res.is_some() {
                let InputGeneric(data, used) = res.unwrap();
                buf.copy_within(used..*n, 0);
                *n -= used;
                return Ok(Some(data));
            }
        }
        let x = stream.read(&mut buf[*n..])?; // read a bit more data.
        if x == 0 {
            return Ok(None);
        }
        *n += x;
    }
}
1 Like
5

Thank you very much @kpreid and @drewtato!!
I think I need to read more about for<'a>... I don't recall reading about it anywhere.

Wow, it seems I'm in some dark corner of the compiler... When bounding TryFrom::Error: 'static, it does work the method but now fails at the call site saying that "this is a known limitation of the trait solver that will be lifted in the future":

error[E0277]: `<InputGeneric<_> as std::convert::TryFrom<&'a [u8]>>::Error` doesn't implement `std::fmt::Debug`
   --> src/main.rs:159:28
    |
159 |         let err = match ds.wait_next(&mut stream) {
    |                            ^^^^^^^^^ `<InputGeneric<_> as std::convert::TryFrom<&'a [u8]>>::Error` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
    |
    = help: the trait `for<'a> std::fmt::Debug` is not implemented for `<InputGeneric<_> as std::convert::TryFrom<&'a [u8]>>::Error`
note: this is a known limitation of the trait solver that will be lifted in the future
   --> src/main.rs:159:28
    |
159 |         let err = match ds.wait_next(&mut stream) {
    |                            ^^^^^^^^^ try adding turbofish arguments to this expression to specify the types manually, even if it's redundant
note: required by a bound in `DataStream::<N>::wait_next`
   --> src/main.rs:200:57
    |
196 |     fn wait_next<T>(&mut self, stream: &mut TcpStream) -> Result<Option<T>, std::io::Error>
    |        --------- required by a bound in this associated function
...
200 |         for<'a> <InputGeneric<T> as TryFrom<&'a [u8]>>::Error: std::fmt::Debug + 'static,
    |                                                         ^^^^^^^^^^^^^^^ required by this bound in `DataStream::<N>::wait_next`

It is also curious that it states Debug is not implemented for TryFrom::Error, but it is... This is the final version:

    fn wait_next<T>(&mut self, stream: &mut TcpStream) -> Result<Option<T>, std::io::Error>
    where
        InputGeneric<T>: for<'a> TryFrom<&'a [u8]>,
        T: std::fmt::Debug,
        for<'a> <InputGeneric<T> as TryFrom<&'a [u8]>>::Error: std::fmt::Debug + 'static,

So, should I actually call it with ds.wait_next::<Data>(&mut stream)?

6

Here it is, I was able to simulate it on the playground!

Without the std::fmt::Debug on the TryFrom::Error it does work. It only fails when adding both std::fmt::Debug and 'static.

EDIT: It is curious that even removing 'static now, it fails the same way:

error[E0277]: `<InputGeneric<_> as TryFrom<&'a [u8]>>::Error` doesn't implement `Debug`

But if I do put the turbofish as in ds.wait_next::<Data>(&mut stream), then it fails again with the original borrow checking error.

7

It would fail both ways, but the compiler doesn't get to the borrow checking stage.

If you need to use Debug in the function, another thing you can do is add another generic:

fn wait_next<T, E>(&mut self, stream: &mut TcpStream) -> Result<Option<T>, std::io::Error>
where
    InputGeneric<T>: for<'a> TryFrom<&'a [u8], Error = E>,
    E: std::fmt::Debug,

The compiler is okay with this even without the turbofish. I went with the one-generic version since it didn't introduce an unnecessary generic, but if that causes issues, I'd go with the two-generic version. You can also add 'static if you need to debug print it after reusing buf.

2 Likes
8

Wow, amazing! Thank you.
Why is the compiler okay with it if we add a second generic??
And why exactly does it need it with one?

It does work without the turbofish this way:

    fn wait_next<T, E>(&mut self) -> Result<Option<T>, std::io::Error>
    where
        InputGeneric<T>: for<'a> TryFrom<&'a [u8], Error = E>,
        T: std::fmt::Debug,
        E: std::fmt::Debug + 'static,
9

A generic type variable like E represents one concrete type, and types that differ only by lifetime are still distinct types. So this:

fn wait_next<T, E>(&mut self, stream: &mut TcpStream) -> Result<Option<T>, std::io::Error>
where
    InputGeneric<T>: for<'a> TryFrom<&'a [u8], Error = E>,
    E: std::fmt::Debug,

requires that the error type is the same, no matter what 'a is, which in turn means that E can't contain some reborrow of the &'a. It can't "capture" 'a. (It can still capture T, so adding + 'static too is still more restrictive.)

For the turbofish you can do

ds.wait_next::<Data, _>(&mut stream) {
1 Like
10

Ohh, that's very interesting, thanks @quinedot!!
Now it does make sense that the Error can't capture 'a anymore, only T which can be an &T or non-'static itself.

11

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.