Trying to understand lifetimes in loops

Purely as part of my baby steps into Rust-land, I'm trying to understand how to achieve something simple in a loop without falling foul of the borrow checker. Here's the code I'm having trouble with:

use std::io;

enum Error<'a> {
    Invalid(&'a [u8]),  // I put this reference in as part of playing around with lifetimes
    Io(io::Error),
}

struct Thing {
    counter: usize
}

enum Result<'a> {
    Done,
    Error(Error<'a>),
    Value(usize)
}

impl Thing {
    fn new() -> Self {
        Self { counter: 0 }
    }

    fn process(&mut self) -> Result {
        self.counter += 1;
        Result::Done // for now
    }
}

fn main() {
    let mut thing = Thing::new();
    loop {
        let result = thing.process(); 
        match result {
            Result::Done => {
                break;
            }
            Result::Error(e) => panic!(e),
            Result::Value(v) => {
                println!("{}", v);
            }
        }
    }
}

(Playground)

Errors:

   Compiling playground v0.0.1 (/playground)
error[E0499]: cannot borrow `thing` as mutable more than once at a time
  --> src/lib.rs:32:22
   |
32 |         let result = thing.process(); 
   |                      ^^^^^ mutable borrow starts here in previous iteration of loop
   |
   = note: borrowed value must be valid for the static lifetime...

error: aborting due to previous error

For more information about this error, try `rustc --explain E0499`.
error: Could not compile `playground`.

To learn more, run the command again with --verbose.

I sort of understand that the first time round the loop the borrow is given but not handed back, so it would fail the next time around the loop. I have two questions:

  • How do I arrange things so I can successfully mutate the thing in a loop? What do I need to do to "return the borrow"?
  • Why does it say the borrowed value must be valid for the static lifetime, rather than the end of the loop?
3 Likes

This is absolutely evil. You need to change line 37 to:

Result::Error(_) => panic!("e"),

The key to this is the exact language of the error message:

borrowed value must be valid for the static lifetime...

That confused the hell out of me, because there are no static borrows anywhere in this code. What's actually happening is that when you do panic!(e), you're using e as the literal panic payload. In order for that to work, e itself must live forever. But, Error contains a borrow. So that borrow must live forever.

But the 'a lifetime of that Error borrow is tied to the 'a lifetime in Result. And that lifetime is tied to the lifetime of self, which is in turn the required lifetime of the thing variable.

So thing must be 'static, but it cannot be 'static because it's on the stack.

Boom.

The better solution is to implement Display for Error and then use panic!("{}", e).

Edit: Seriously, this is probably the most baffling borrowing issue I've ever seen. Not only does the error message say something that makes no sense, it doesn't even remotely point at the actual culprit.

13 Likes

Well, thanks for restoring my sanity. I thought I was going doolally.

Does this merit a bug report for the confusing error message?

I don't know how you would improve it. In most cases, it's probably fine, but here the 'static requirement is kind of sneaking in via the panic!, then transitively carrying backward to the initial borrow.

I suppose it could be improved if the compiler could remember (and then subsequently identify) specifically where broader requirements are introduced. Not sure how tractable that is, though.

Is there some super-duper diagnostic mode that enables you to see more of the internal decision making regarding the borrow checker? Not that you'd want the info constantly in your face, but for situations like this ...

None that I'm aware of.

That is indeed a fiddly one! Nice diagnosis!

Well, the sane thing to do would be to change the referenced slice [u8] in the Error to be an owned Vec<u8>. Then all lifetime problems go away, at the cost of allocating :smile:

Specifically for Errors, this makes intuitive sense, and seems to be the recommended style:l.
Errors are usually propagated "up", so out of the current scope, but their nature as errors requires them to log details about inner state.
You usually don't want to keep pointing into known-bad state, and it is hard to keep that known-bad state alive just for the sake of logging its details.
All that combines into the sensible conclusion that an error should be wholly owned.
Where it is generated, it should make a copy of all the internal state that needs keeping, and then just let the trainwreck that spawned it burn down on its own; our Owned Error then goes upwards to inform the caller about said trainwreck.

All the error libraries I am aware of so far have Owned error types, for exactly this reason.

4 Likes

All good pointers (no pun intended :wink:), for which many thanks. It's only a slice in this example because I'm playing around.

1 Like

I think it's great that you are playing around like this! Really helps to understand reality, instead of only knowing how something works in theory.
I just wanted to make clear what the "real life" solution would be. The answer would definitely be different if the struct would be named "ProcessedData" instead of "Error" :smile:

I'd say it does; "confusing errors/unhelpful compiler" are first class bugs in Rust :slight_smile:
I'm not quite sure how the compiler can do better, apart from a massive rewrite of the inference logic, but it's not like that isn't exactly what we're doing currently with "codename Polonius" (non-lexical lifetimes)..

Before you go reporting however, could you try if the error message changes in the 2018 edition preview?
From the top of my head (I'm on mobile, so cant easily look it up) I believe just using the nightly compiler should do it, otherwise you'll need to add edition = 2018 to cargo.toml

This is getting off topic, but: I don't subscribe to that approach. Largely for three reasons: first of all, sometimes the context of an error is really big. I don't want to be making huge copies of stuff because a few extra lifetimes are a bit inconvenient. Secondly: you sometimes don't know if an error is actually fatal or not. I don't want to be doing a lot of work constructing an error value that's just going to be immediately discarded. Finally: sometimes an error doesn't even have complete context. In those cases, I can't meaningfully display the error without feeding in additional context later on, so I'm going to have to do some level of "error threading" anyway.

If it's unavoidable, or an unbearable pain in the sitting hardware, I'll use Cow (or something similar) with an into_owned method as an escape hatch.

Personally, I view errors that clone and own their context as a... failing of implementation and design.

2 Likes

And I think this demonstrates the occasional unpleasantness of macros. Consider if panic were a function - it would have the lifetime requirement in its signature, eg something like:

fn panic<T: 'static>(payload: T) -> ! {...}

Here it’d be easier to see where the 'static bound is coming from. Macros, on the other hand, cover up a bunch of arbitrary code, and obviously don’t convey any type or lifetime bounds in their “signature”.

4 Likes

Excellent arguments! There is definitely a spectrum there, and not every small error should go wildly copying everything.

The hypothetical error here was being passed all the way to top-level panic in main, so there I think it is clear-cut.
If you're doing smaller errors in hotter loops, or something that is just a signal for one level up, I fully agree we shouldn't be copying everything, or even anything.

I believe polonius will be able to accomplish that. The error messages in a future Rust should be amazing :slight_smile:

1 Like

I shall try it now and post the results.
Edit- it's the same

Ok, then I believe it deserves a bug report :slight_smile:

Do you have a GitHub account? Otherwise I can post it on your behalf.

If you post it yourself, be sure to include your example program, the error message, and a copy of DanielKeep's diagnosis. Preferably a link to this thread as well.
(Copying all the information into one place will help the bug-squashers work more efficiently)

Yes I do, and I will post it soon.

Update: New issue submitted.