How do I tell Rust to "trust me" ... it's only moved once?

Based on my pest parsed data, it seems my code should only move the "ident" String once each time through the inner loop (although I'm a newbie). But how do I convince Rust without resorting to having to clone "ident" variable. The rule bitSize in the code snippet should only fire once in the inner loop so "ident" should only be assigned to new_record (moved) once. But it seems Rust presumes worst case. Is there a way around this other than cloning the ident string? Or did I miss something?

... snippet ...
for pair in pairs.into_inner() {
    match pair.as_rule() {
        Rule::record => {
            let mut ident : String;
            let mut rec_bit_size : i32;
            let mut new_record : OdbEnum;

            for field in pair.into_inner() {
                match field.as_rule() {
                    Rule::identifier => {
                        ident = field.as_str().to_string();
                    }
                    Rule::bitSize => {
                        rec_bit_size = field.as_str().parse().unwrap();
                        new_record = OdbEnum::Record{name: ident, size: rec_bit_size, elements: Vec::new()};
                    }
                    _ => (),
                }
            }
        }
        _ => (),
    }
}


66 |        new_record = OdbEnum::Record{name: ident, size: rec_bit_size, elements: Vec::new()};
   |                                           ^^^^^ value moved here, in previous iteration of loop

Borrow checker only works on the shape of code, not values. Every loops, even while false {} ones, considered that it can run more than once.

If you're sure that moving never happens more than once, you can defer the move check to the runtime by using Option<T>. Just assign it using = Some(value) and take it as .take().unwrap(). If the assumption is correct the code would works well. If not, maybe you've made some mistake, it crashes as usual. If in the final code it's really obvious that the variable only taken once, the optimizer may elide the flag part of the Option<T>.

8 Likes

In this case I recommend using mem::take rather than an Option.

new_record = OdbEnum::Record{name: mem::take(ident), size: rec_bit_size, elements: Vec::new()};

This leaves an empty string in ident, which is very cheap and does not allocate.

Note that even if you could convince the compiler that it is only moved once, it would still need a flag of some sort so it knows whether to run the destructor or not. Interestingly, using mem::take avoids this, as it can always run the destructor.

4 Likes

Would it be feasible to stop the loop, when you hit Rule::bitSize? If yes, you should be able to add a break in the match arm and be good to go

2 Likes

The advantage of using Option is that if you happen to be wrong about the assumption that ident is only moved once, or if the rules come out of order, it will panic at the unwrap() instead of silently proceeding with an empty string in ident.

A cursory glance over the docs for pest was not able to confirm that this assumption is always correct, so I'd prefer to use Option.

6 Likes

FWIW, I wouldn't be a fan of this code even in a looser language where it compiled.

It's relying on non-obvious postconditions of the fields in the pairs:

  • There is always at least one Rule::identifier before any Rule::bitSizes.
    • And ignoring all but the last of them.
  • There is at most one Rule::bitSize.

So this isn't quite the FOR-CASE antipattern, but it's close.

My instinct here is to make it so that pairs can return a struct of real fields, not an iterator of them.

1 Like

As @scottmcm said, a probably simpler way if possible is to unroll the last loop:

           let mut pair = pair.into_inner() ;

           // get ident first
           let ident = pair.next().and_then(|field| if let Rule::identifier = field.as_rule() {
               Some(field.as_str().to_string())
           } else {
               None
           }).expect("Cannot get ident");

           // continue ...
           for field in pair {
                match field.as_rule() {
                    Rule::identifier => {
                        panic!("identifier already set!")
                    }
                    Rule::bitSize => {
                        rec_bit_size = field.as_str().parse().unwrap();
                        new_record = OdbEnum::Record{name: ident, size: rec_bit_size, elements: Vec::new()};
                    }
                    _ => (),
                }
            }