24

I'm trying to implement a commonly used pattern - using the result of a previous loop iteration in the next loop iteration. For example, to implement pagination where you need to give the id of the last value on the previous page.

struct Result {
    str: String,
}    

fn main() {
    let times = 10;
    let mut last: Option<&str> = None;

    for i in 0..times {
        let current = do_something(last);
        last = match current {
            Some(r) => Some(&r.str.to_owned()),
            None => None,
        };
    }
}

fn do_something(o: Option<&str>) -> Option<Result> {
    Some(Result {
        str: "whatever string".to_string(),
    })
}

However, I'm not sure how to actually get the value out of the loop. Currently, the compiler error is temporary value dropped while borrowed (at &r.str.to_owned()), though I made many other attempts, but to no avail.

The only way I found to actually get it working is to create some sort of local tmp_str variable and do a hack like this:

match current {
    Some(r) => {
        tmp_str.clone_from(&r.str);
        last = Some(&tmp_str);
    }
    None => {
        last = None;
    }
}

But that doesn't feel like it's the way it's supposed to be done.

ralh
  • 2,514
  • 1
  • 13
  • 19
  • I asked a concise Question for common rust error `error[E0716]` [error E0716: temporary value dropped while borrowed (rust)](https://stackoverflow.com/questions/71626083/error-e0716-temporary-value-dropped-while-borrowed-rust). It links back to this Question. – JamesThomasMoon Mar 26 '22 at 07:27
  • This feels like an unnecessary error on Rust's part when `#to_owned` is involved. – Eric Walker Aug 05 '22 at 21:17

2 Answers2

14

In your code, it remains unclear who the owner of the String referenced in last: Option<&str> is supposed to be. You could introduce an extra mutable local variable that owns the string. But then you would have two variables: the owner and the reference, which seems redundant. It would be much simpler to just make last the owner:

struct MyRes {
    str: String,
}

fn main() {
    let times = 10;
    let mut last: Option<String> = None;

    for _i in 0..times {
        last = do_something(&last).map(|r| r.str);
    }
}

fn do_something(_o: &Option<String>) -> Option<MyRes> {
    Some(MyRes {
        str: "whatever string".to_string(),
    })
}

In do_something, you can just pass the whole argument by reference, this seems more likely to be what you wanted. Also note that naming your own struct Result is a bad idea, because Result is such a pervasive trait built deeply into the compiler (?-operator etc).


Follow-up question: Option<&str> or Option<String>?

Both Option<&str> and Option<String> have different trade-offs. One is better for passing string literals, other is better for passing owned Strings. I'd actually propose to use neither, and instead make the function generic over type S that implements AsRef<str>. Here is a comparison of various methods:

fn do_something(o: &Option<String>) {
    let _a: Option<&str> = o.as_ref().map(|r| &**r);
    let _b: Option<String> = o.clone();
}
fn do_something2(o: &Option<&str>) {
    let _a: Option<&str> = o.clone(); // do you need it?
    let _b: Option<String> = o.map(|r| r.to_string());
}
fn do_something3<S: AsRef<str>>(o: &Option<S>) {
    let _a: Option<&str> = o.as_ref().map(|s| s.as_ref());
    let _b: Option<String> = o.as_ref().map(|r| r.as_ref().to_string());
}

fn main() {
    let x: Option<String> = None;
    let y: Option<&str> = None;

    do_something(&x);                           // nice
    do_something(&y.map(|r| r.to_string()));    // awkward & expensive

    do_something2(&x.as_ref().map(|x| &**x));   // cheap but awkward
    do_something2(&y);                          // nice

    do_something3(&x);                          // nice
    do_something3(&y);                          // nice, in both cases
}

Note that not all of the above combinations are very idiomatic, some are added just for completeness (e.g. asking for AsRef<str> and then building an owned String out of seems a bit strange).

Andrey Tyukin
  • 43,673
  • 4
  • 57
  • 93
  • Thanks for the answer. I have some more follow up questions. From what I understand, one should mostly use `&str` and not `String` in function arguments. When using some form of `Option`, is the best way to mostly just pass around `&Option`? Nothing gets copied then, right? And in your answer, wouldn't it be better to just have `last: Option`? Does it have any downsides, except memory usage I guess? And yeah, `Result` was obviously a toy example, though indeed perhaps a bad one. – ralh Feb 02 '19 at 00:06
  • 1
    @ralh As far as I understand - no, nothing gets copied. Essentially, only a single pointer is passed to `do_something`, and it is also guaranteed that `do_something` won't modify the content of `_o` in any way. If you have to, you can convert `Option` into `Option<&str>` by `as_ref()` followed by a `map` with coercion, as [explained here](https://stackoverflow.com/questions/31233938/converting-from-optionstring-to-optionstr). I would actually propose to use neither `&str` nor `String`. I added a comparison of the various methods. – Andrey Tyukin Feb 02 '19 at 00:26
  • Thanks again for the very detailed answer! I knew there was probably some Rust type magic to be done here. I wonder - does the `AsRef` method add any overhead (since you're calling `as_ref`, `map`...), or is Rust smart enough to compile such overhead away? – ralh Feb 02 '19 at 09:05
  • 3
    @ralh as far as I understand it, rustc has no other choice but to optimize it all away: `Option` is doing something called ["null pointer optimization"](https://stackoverflow.com/questions/46557608/what-is-the-null-pointer-optimization-in-rust), so that all `Option`-related stuff is eliminated here. The type `S` is inferred statically at compile time, no dynamic dispatch takes place anywhere, so there is no object on which one could actually call `map` at runtime. So, all of the above seems to be a purely compile-time game with the typechecker, it leaves no traces after the compilation. – Andrey Tyukin Feb 02 '19 at 09:47
  • 1
    `Option<&str>` and `Option` can take advantage of null pointer optimization, but you're still incurring a double indirection cost when you use `&Option<...>`. And it's more difficult to call a function taking `&Option` when you have a `T` in hand, because you may have to move the `T` first. For those reasons I would write `do_something3` to take `Option<&S>` instead of `&Option`. You can then call `do_something(x.as_ref())` (if you don't want to consume a `String`) or `do_something(y)` (since `&` references are `Copy`, you don't need a non-consuming version). – trent Feb 02 '19 at 17:02
  • @ralh A corollary to the above comment is that it actually depends on what you're doing with the value inside `do_something`; if you are going to use a `String`, you should accept a `String` (or `S: Into`) and let the caller decide whether to clone or not -- that's more flexible for the caller than accepting a reference which you then unconditionally copy. That's why I suggested `Option` in my answer. There are cases where you may want either one. – trent Feb 02 '19 at 17:13
  • @trentcl On *"it actually depends on what you're doing with the value inside do_something"* - would it maybe make sense to ask a separate question with a list of all the different variants, and then explain step by step what is useful in which situations, and then convert the follow-up answer and the discussion in the comments into one big separate answer? – Andrey Tyukin Feb 02 '19 at 19:46
  • Sure, that sounds like a good idea. I was looking for a question like that to link to but couldn't find one. – trent Feb 02 '19 at 21:55
4

r.str.to_owned() is a temporary value. You can take a reference to a temporary, but because the temporary value will usually be dropped (destroyed) at the end of the innermost enclosing statement, the reference becomes dangling at that point. In this case the "innermost enclosing statement" is either the last line of the loop, or the loop body itself -- I'm not sure exactly which one applies here, but it doesn't matter, because either way, you're trying to make last contain a reference to a String that will soon be dropped, making last unusable. The compiler is right to stop you from using it again in the next iteration of the loop.

The easiest fix is just to not make last a reference at all -- in the example, it's not necessary or desirable. Just use Option<String>:

fn main() {
    let times = 10;
    let mut last = None;

    for _ in 0..times {
        last = match do_something(last) {
            Some(r) => Some(r.str),
            None => None,
        };
    }
}

fn do_something(_: Option<String>) -> Option<Result> {
    // ...
}

There are also ways to make the reference version work; here is one:

let mut current;  // lift this declaration out of the loop so `current` will have
                  // a lifetime longer than one iteration
for _ in 0..times {
    current = do_something(last);
    last = match current {
        Some(ref r) => Some(&r.str),  // borrow from `current` in the loop instead
                                      // of from a newly created String
        None => None,
    };
}

You might want to do this if your code is more complicated than the example and using String would mean a lot of potentially expensive .clone()s.

trent
  • 25,033
  • 7
  • 51
  • 90