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);
}
}
}
}
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?
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.
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 ...
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
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.
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"
I'd say it does; "confusing errors/unhelpful compiler" are first class bugs in Rust
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.
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”.
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.
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)