How to end borrow in this code?

help
1

I try to implement approach for testing that was advised in that answer. I plan to use this approach in the GitHub - tafia/quick-xml: Rust high performance xml reader and writer project. Below the simplified code from quick-xml and test code that I try to write for https://github.com/mzabaluev/generic-tests:

use std::io::BufRead;

//---------------------------------------------------------------
// quick-xml code
trait BufferedInput<'b, B> {
  fn read(&mut self, buf: B) -> &'b [u8];
}

/// Generic reader, copies data from reader into intermediate buffer
impl<'b, R: BufRead> BufferedInput<'b, &'b mut Vec<u8>> for R {
  /// Read & Copy data from `self` into `buf`,
  /// return data that borrow from `buf`
  fn read(&mut self, buf: &'b mut Vec<u8>) -> &'b [u8] {
    &buf[..]
  }
}

/// Borrowed reader, zero-copy
impl<'i> BufferedInput<'i, ()> for &'i [u8] {
  /// Directly borrow data from `self`, do not copy,
  /// return data that borrow from `self`
  fn read(&mut self, _buf: ()) -> &'i [u8] {
    self
  }
}

//---------------------------------------------------------------
// My tests
#[cfg(test)]
mod test {
  trait Storage<'b>: Default {
    type Buffer;

    /// Returns buffer for `BufferedInput::read`
    fn buffer(&'b mut self) -> Self::Buffer;
  }
  impl<'b> Storage<'b> for () {
    type Buffer = ();
    fn buffer(&'b mut self) {}
  }
  impl<'b> Storage<'b> for Vec<u8> {
    type Buffer = &'b mut Vec<u8>;
    fn buffer(&'b mut self) -> Self::Buffer {
      self
    }
  }

  // function for `generic-tests` crate
  fn test<'b, S: 'b + Storage<'b>>()
    where &'static [u8]: super::BufferedInput<'b, S::Buffer>,
  {
    use super::BufferedInput;

    // Create storage
    let mut storage = S::default();

    // Get the buffer. Depending on the `S`,
    // that buffer can borrow from `storage` or not
    let buf = storage.buffer();

    let mut input = b"".as_ref();
    input.read(buf);
  }

  // functions to what `generic-tests` crate should expand `test` function
  #[test]
  fn buffered_test() {
    test::<Vec<u8>>();
  }
  #[test]
  fn borrowed_test() {
    test::<()>();
  }
}

Playground

But this code cannot compile because of strange error from borrow checker that I don't understand:

   Compiling playground v0.0.1 (/playground)
error[E0597]: `storage` does not live long enough
  --> src/lib.rs:59:15
   |
49 |   fn test<'b, S: 'b + Storage<'b>>()
   |           -- lifetime `'b` defined here
...
59 |     let buf = storage.buffer();
   |               ^^^^^^^^^^^^^^^^
   |               |
   |               borrowed value does not live long enough
   |               argument requires that `storage` is borrowed for `'b`
...
63 |   }
   |   - `storage` dropped here while still borrowed

For more information about this error, try `rustc --explain E0597`.
error: could not compile `playground` due to previous error

I don't understand why storage cannot be dropped and who is borrowed it? Even if instead of using buf in BufferedInput::read I just drop() it, nothing changed. I do not see any other borrows here. So what the borrow checker tries to say me here?

Also, I don't understand why I need the S: 'b + ... bound in the line 49, but without it the following error is produced (Playground):

   Compiling playground v0.0.1 (/playground)
error[E0309]: the parameter type `S` may not live long enough
  --> src/lib.rs:59:15
   |
49 |   fn test<'b, S: Storage<'b>>()
   |               -- help: consider adding an explicit lifetime bound...: `S: 'b +`
...
59 |     let buf = storage.buffer();
   |               ^^^^^^^ ...so that the type `S` is not borrowed for too long

error[E0309]: the parameter type `S` may not live long enough
  --> src/lib.rs:59:23
   |
49 |   fn test<'b, S: Storage<'b>>()
   |               -- help: consider adding an explicit lifetime bound...: `S: 'b +`
...
59 |     let buf = storage.buffer();
   |                       ^^^^^^ ...so that the reference type `&'b mut S` does not outlive the data it points at

For more information about this error, try `rustc --explain E0309`.
error: could not compile `playground` due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
error: build failed

If I understand Storage trait correctly, the buffer() method should return buffer with the same lifetime that storage have and storage lifetime should determine the final lifetime. Because this sounds like a candidate for HRTB, I tried to change test bounds to:

// function for `generic-tests` crate
fn test<S>()
  where for<'b> S: Storage<'b>,
        &'static [u8]: super::BufferedInput<'b, S::Buffer>,
{
  use super::BufferedInput;

  // Create storage
  let mut storage = S::default();

  // Get the buffer. Depending on the `S`,
  // that buffer can borrow from `storage` or not
  let buf = storage.buffer();

  let input = b"".as_ref();
  input.read(buf);

  drop(buf);
}

But it seems impossible to use one for<'b> for both constraints, and using for<'b> for each constraint independently produces another error (playground):

   Compiling playground v0.0.1 (/playground)
error: implementation of `BufferedInput` is not general enough
  --> src/lib.rs:73:5
   |
73 |     test::<()>();
   |     ^^^^^^^^^^ implementation of `BufferedInput` is not general enough
   |
   = note: `BufferedInput<'0, ()>` would have to be implemented for the type `&'static [u8]`, for any lifetime `'0`...
   = note: ...but `BufferedInput<'1, ()>` is actually implemented for the type `&'1 [u8]`, for some specific lifetime `'1`

error: could not compile `playground` due to previous error

So I tried to move &'static [u8]: super::BufferedInput<'b, S::Buffer> constraint into Storage trait itself, but that also produces errors (at least, I understand them and they legitimate).

So my questings is: why the original errors are produced and is there any way to solve this problem?

2

By changing your trait definition to:

// Hack: introduce an implicit `where Self : 'b` bound
trait BufferedInput<'b, B, _whereSelfOutlivesb = &'b Self> {
    fn read(&'_ mut self, buf: B) -> &'b [u8];
}

then something like:

        for<'local>
            &'static [u8] : BufferedInput<'local, <S as Storage<'local>>::Buffer>
        ,

will yield a constraint that only uses small enough 'local lifetimes,

and from there:

impl<'i, 'j : 'i> BufferedInput<'i, ()> for &'j [u8] {
    /// Directly borrow data from `self`, do not copy,
    /// return data that borrow from `self`
    fn read(&'_ mut self, _buf: ()) -> &'i [u8] {
        self
    }
}

yields a correctly higher-order bound for &[u8].

By the way, don't worry if the solution does not seem obvious to come by "alone", it's a kind of messy area of the language which will get better / clearer once we have generic_associated_types :slightly_smiling_face:

1 Like
3

The lifetime 'b is an input parameter to your fn test, and that means that the caller determines 'b. So long as the other bounds are met, they could call test with a lifetime that's longer than the function body, e.g. they could call it with 'static. But you're trying to borrow from a local variable, so you can't actually support such a lifetime. You can only support a lifetime that, from the implementer of Storage's perspective, is arbitrarily short.

The way one usually writes a bound like that is to not accept a specific lifetime, and instead require that a trait be implemented for any lifetime; if that is true, then you are free to use any lifetime that works within your function. And this is exactly what you tried:

  fn test<S: for<'any> Storage<'any>>()
    where &'static [u8]: for<'any> super::BufferedInput<'any, <S as Storage<'any>>::Buffer>,

And as you noted, this allows your function to compile, but these bounds cause one if your test cases to not compile.

   = note: `BufferedInput<'0, ()>` would have to be implemented for the type `&'static [u8]`, for any lifetime `'0`...
   = note: ...but `BufferedInput<'1, ()>` is actually implemented for the type `&'1 [u8]`, for some specific lifetime `'1`

And, sure enough, quick-xml has only implemented BufferedInput<'static, ()> for &'static [u8]:

impl<'i> BufferedInput<'i, ()> for &'i [u8] {

And this seems reasonable, you can't return a &'static [u8] if you're a &'something_shorter [u8] after all. But we'll come back to this later.

We can connect this current state back to your original bounds error. Remember, with those bounds, the caller can choose the lifetime, so long as the other bounds are met. And the only way for this now-failing caller to have called your original fn test is by choosing the lifetime 'static for 'b, so that the BufferedInput bounds gets met. That in turn would have required borrowing your local S for a 'static lifetime, when it's merely a local variable.

You can't have the caller choosing the lifetime anyway, but maybe this makes the reason why more clear.

If instead of requiring a static buffer &[u8], you allow yourself to choose any lifetime for the buffer, everything compiles and passes tests.

    fn test<S>()
    where
        for<'b> S: Storage<'b>,
        for<'b> &'b [u8]: super::BufferedInput<'b, <S as Storage<'b>>::Buffer>,

But I'm not sure that's actually what you want.

Let's go back to this part:

impl<'i> BufferedInput<'i, ()> for &'i [u8] {
    fn read(&mut self, _buf: ()) -> &'i [u8] {
        self
    }
}

While it's true you can't borrow a 'static from something shorter, you can borrow something shorter from a 'static:

// `&'static [u8]` in particular implements
// `for<'any> BufferedInput<'any, ()>`
impl<'long: 'short, 'short> BufferedInput<'short, ()> for &'long [u8] {
    fn read(&mut self, _buf: ()) -> &'short [u8] {
        self
    }
}

And this means that &'static [u8] can meet your bounds now. I'm still not entirely sure that this is what you want -- the way your Storage trait is written, you're always limited by the lifetime of the local S -- but I think it's at least closer.

However, this is a change to the quick-xml code, not yours. But it's a very reasonable one to request.

You may wonder why we had to spell this out, and the answer is that lifetimes in trait parameters are invariant -- they can't automatically shrink, like you're used to with &[u8], say. The above change makes the implementation covariant by breaking up the lifetime of the implemented trait ('short) and the lifetime of the implementer ('long) -- every concrete &'long [u8] gets an implementation for every 'short that is shorter.

Just an aside to explain the immediate error, an implementer of Storage<'b> has to implement buffer that takes a &'b mut Self. If Self wasn't valid for at least 'b, this would require taking a reference that lasts longer than the underlying object. Rust is ok with you not requiring this at the definition of the trait, because it won't be allowed elsewhere, and maybe your trait has other uses... so it only complained when you tried to call buffer. In this case, your trait is useless without that bound, so you could just add it to the trait: trait Storage<'b>: Default + 'b. Then you don't have to mention it elsewhere.

It might be cleaner to break up Storage into two parts, one with a lifetime that defines the type, and another with no lifetime that defines the function. But I'll leave it here for now.

3 Likes
4

Fwiw, dpending on the specificities of the test code involved, a helper trait could "fix" the bounds without requiring changes upstream

5

Thank you for your detailed answer and explanations!

Initially I chosen &'static [u8] because I thought that it will be enough because I only use 'static b"byte strings" in my tests and I wrongly assumed if I write constraint as &'b [u8]: super::BufferedInput<'b, <S as Storage<'b>>::Buffer> it always will match BufferedInput<'b, ()> impl. But now I've checked output and it proves that each test function calls the right implementation: playground. So you change is what I want.

Anyway, I write this code for my PR to quick-xml, so changes in it itself is acceptable. Although, split these lifetimes will really be a good idea, even if now it is not very required.

6

Wow! Thanks again -- this is the second time you've helped me out. Although in the end the @quinedot solution turned out to be easier and without hacks. But at least I learnt some dark force...

7

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.