Borrow error when adding lifetime field to a Result::Err

[keithduncan]

I’m trying to add context to one of my errors by adding a reference to an enum case returned from a function fn read_line<'a>(&mut self, buf: &'a mut [u8]) -> Result<(), ParseError<'a>>. Doing so causes the calling method to fail to compile with E0499 due to borrowing buf as mutable more than once and I’m struggling to see why.

I’ve minimally reproduced the issue in Rust Playground.

What I’m trying to do is read bytes from the &mut Read and store them in &mut buf. In practice this is a #![no_std] crate so I can’t allocate. buf is a stack allocated buffer passed to my modem driver for it to store Hayes AT command responses in and &mut Read is a backed by a ring buffer serviced by a hardware UART interrupt.

The parser reads line by line and stops once it hits a terminating line (OK or ERROR etc). If the final response line isn’t OK I want to return an Err(ParseError::Line(the_error_line)) that references bytes from the buffer that was passed in to the parser.

This seems like it should work; the parser receives a borrow and either returns Ok(offset), no borrows in the value (though there is a borrow in the type, is the entire Result tagged with 'a?) , or Err(ParseError<'a>) where the provided mutable borrow should be extended until this returned value is dropped.

I’ve never fully mastered rust lifetimes and am hoping someone can point me in the right direction.

[RustyYato]

Instead of storing a reference to a slice in the error condition, why not store an index and remove the lifetime? Right now Rust has trouble proving that the lifetimes here are fine because of the return branch inside of the ? operator.

Also your use of from_utf8_unchecked isn't sound and will lead to memory bugs if the line is not valid utf-8, use from_utf8 instead.

[keithduncan]

Thanks denormalising the slice bounds into enum fields ParseError::Line{ start: usize, end: usize } does work though I would prefer to maintain a borrow of buffer in the error if possible.

I can reconstruct the slice using []::get() in the error handler but would have to handle the new None case, if the slice can be returned natively this case doesn’t exist to be handled. Returning bounds would also allow the buffer to be reused or modified between the error being returned and the slice being reconstructed. I think we can guarantee the slice holds the same content by maintaining a borrow?

Right now Rust has trouble proving that the lifetimes here are fine because of the return branch inside of the ? operator

Would there be another way to write or annotate this to help Rust follow the lifetimes?

[RustyYato]

@keithduncan

Would there be another way to write or annotate this to help Rust follow the lifetimes?

Not at the moment, Rust has trouble with the branch inside of the ?, and desugaring it doesn't help. We will have to wait and see if polonius fixes the problem when it arrives.

[Hyeonu]

No, this is not the case polonius can help. Let's see the code.

    fn read_line<'b>(&mut self, buf: &'b mut [u8], mut offset: usize) -> Result<usize, ParseError<'b>> { .. }

    pub fn parse_lines<'b>(&mut self, buf: &'b mut [u8]) -> Result<(), ParseError<'b>> {
        let offset = 0;
        let offset = self.read_line(buf, offset)?; // line 1
        let _offset = self.read_line(buf, offset)?; // line 2
        Ok(())
    }

At the line 1, we can return Err(ParseError<'b>) here. And in the signature of read_line the return type of it has same lifetime of the buf: &mut [u8]. This means at the line 1, the buf is already borrowed with the entire 'b lifetime, so it cannot be used again in this function!

[keithduncan]

@Hyeonu

so it cannot be used again in this function!

This is the part that confuses me, in the event that read_line returns Err() and propagates the borrow parse_lines doesn’t use buf again, it returns.

Only in the case where read_line returns Ok() where there is no borrow is buf used a second or more times.

Does having the 'b lifetime in the return type mean that any value returned carries that lifetime, even if the borrow is only present in some inhabitants of the type?

[Yandros]

fn parse_lines<'buf>(
    self: &'_ mut Self,
    buf: &'buf mut [u8],
) -> Result<(), ParseError<'buf>>
{
    let offset = 0;
    
    let offset = self.read_line(buf, offset)?; // line 1
    let _offset = self.read_line(buf, offset)?; // line 2
    Ok(())
}

becomes

fn read_line<'buf> (
    self: &'_ mut Self,
    buf: &'buf mut [u8],
    offset: usize,
) -> Result<usize, ParseError<'buf>>;

fn parse_lines<'buf>(
    self: &'_ mut Self,
    buf: &'buf mut [u8],
) -> Result<(), ParseError<'buf>>
{
/*1*/   let offset = 0;
/*2*/   let offset = match self.read_line(buf, offset) {
/*3*/       | Ok(offset) => offset,
/*4*/       | Err(err) => return Err(err.into());
/*5*/   };
/*6*/   let _offset = self.read_line(buf, offset)?;
/*7*/   Ok(())
}

So the thing is that at line 2 there is a (re)borrow of buf. For line 6 to be accepted by Rust, this (re)borrow needs to end before it (e.g. at line 5).
However, the return ... err at line 4 returns an error that is borrowing from buf with a lifetime 'buf, as specified in the function signature: the (re)borrow of buf at line 2 must thus be held for the lifetime 'buf, which is the "maximum" known lifetime of buf. There is thus no room left for any further borrow, not even the one at line 6.

The only way to solve this would be for the borrow checker to be smart enough to figure out that the borrow of buf doesn't have to be held for equal durations / lifetimes in the Ok and Err branches: "if read_line fails, then forbid any further borrow in that execution path since such borrow must be held for the input lifetime, but otherwise (in other execution paths) allow further borrows". This is the kind of problems that the next borrow checker, polonius, aims to solve, since the current borrow checker currently clearly does not.

---

Sadly, the only solution without additional runtime cost, if you still want to have an error that borrows from its input, and if using exactly the pattern shown in the OP, is to use unsafe to decorrelate the buf borrow from the returned value:

pub fn parse_lines<'buf> (
    self: &'_ mut Self,
    buf: &'buf mut [u8],
) -> Result<(), ParseError<'buf>>
{
    let offset = 0;
    let offset = unsafe {
        // Safety:
        //   - buf : 'buf
        //   - Ok path does not keep the borrow from buf,
        self.read_line(&mut *(buf as *mut _), offset)?
    };
    let offset = unsafe {
        // Safety:
        //   - buf : 'buf
        //   - Ok path does not borrow from buf,
        self.read_line(&mut *(buf as *mut _), offset)?
    };
    let _offset = unsafe {
        // Safety:
        //   - buf : 'buf
        //   - Ok path does not borrow from buf,
        self.read_line(&mut *(buf as *mut _), offset)?
    };
    Ok(())
}

• Playground (I have taken the liberty to rewrite your parse_line, which no longer assumes valid utf-8, and does not skip checks on the end-of-stream line).

[keithduncan]

Thanks! That all makes sense to me.

It seems reasonable to me to use a small unsafe scope here where an otherwise valid program is rejected due to a generally suitable but conservative assumption in the borrow checker.

I think I’ll use this pattern for the time being and track progress on the new borrow checker.