Grasping the borrow checker

help
1

I am just starting out with my first rust mini project and hit a stumbling block with the borrow checker. The project is to regularly poll some sensors in different "zones" and log the results. But the polled sensor values don't change very often so, to minimize storing the data, I only care about data when it has changed from the previous poll data.

The code below is a simplification of this, but the get_changed_zone_data() function is failing to compile with errors error[E0505]: cannot move out of zone_data because it is borrowed and error[E0515]: cannot return value referencing local variable zone_data.

I tried to fix this with some lifetimes by saying (as far as I understand) that the returned result: Vec<&ZoneData> should not outlive &self, but clearly I have not understood something here.

#[derive(Default, PartialEq)]
enum Zone {
    A,
    B,
    #[default]
    X,
}

#[derive(Default, PartialEq)]
struct ZoneData {
    z: Zone,
    v: u32,
}

struct Zones {
    previous_data: Vec<ZoneData>,
}

impl Zones {
    fn new() -> Zones {
        Zones {
            previous_data: (0..2)
                .map(|_| ZoneData {
                    ..Default::default()
                })
                .collect(),
        }
    }

    fn get_zone_data(&self) -> Vec<ZoneData> {
        // dummy function
        vec![ZoneData { z: Zone::A, v: 1 }, ZoneData { z: Zone::B, v: 1 }]
    }

    fn get_changed_zone_data<'b, 'a: 'b>(&'a mut self) -> Vec<&'b ZoneData> {
        let zone_data = self.get_zone_data();
        let result: Vec<&ZoneData> = zone_data
            .iter()
            .zip(self.previous_data.iter())
            .filter(|(current, previous)| current != previous)
            .map(|(current, _)| current)
            .collect();
        self.previous_data = zone_data;
        result
    }
}

fn main() {
    let mut zones = Zones::new();
    let _ = zones.get_changed_zone_data();
}

(Playground)

I've tried some variations, but not got anywhere close to compiling without just cloning the ZoneData structs in the iter .map(|(current, _)| current). It isn't a big deal to clone these, but it would be helpful to learn if/how this can be made to compile.

All help much appreciated :slight_smile:

R

2

You can't move something that's borrowed. So put your data in place first, and then iterate over it.

    fn get_changed_zone_data(&mut self) -> Vec<&ZoneData> {
        let zone_data = self.get_zone_data();
        let old_data = std::mem::replace(&mut self.previous_data, zone_data);
        self.previous_data
            .iter()
            .zip(old_data.iter())
            .filter(|(current, previous)| current != previous)
            .map(|(current, _)| current)
            .collect()
    }

Also consider doing this instead:

    fn get_changed_zone_data(&mut self) -> impl Iterator<Item = &ZoneData> {
        let zone_data = self.get_zone_data();
        let old_data = std::mem::replace(&mut self.previous_data, zone_data);
        self.previous_data
            .iter()
            .zip(old_data.into_iter())
            .filter(|(current, previous)| *current != previous)
            .map(|(current, _)| current)
    }

(Callers of the method can collect themselves if they want the Vec.)

3

but not got anywhere close to compiling without just cloning the ZoneData structs in the iter

Don't be afraid to clone. Rust wants you to be aware of when you're making copies, but that doesn't mean that making copies is a bad thing. Copying is only a problem when it either impairs the actual performance of your program, or when it changes the semantics of your program in ways you don't intend.

From the point of view of a caller, it's likel that either Vec<ZoneData> or impl Iterator<Item = ZoneData>, yielding one ZoneData per difference between current data and past data, is easier to work with, as well, and that more or less forces copying of one kind or another. You can avoid clone if you have stylistic objections by making your copying more explicit, but it's the same operation.

1 Like
4

well


//make sure Zone and ZoneData both derive the Clone trait so it can be cloned
///////////////////////////////////////////
#[derive(Default, PartialEq, Clone)]
enum Zone {
   ...
}

#[derive(Default, PartialEq, Clone)]
struct ZoneData {
...
}
///////////////////////////////////////////

    fn get_changed_zone_data<'b, 'a: 'b>(&'a mut self) -> Vec<ZoneData> {
        let owned_zone_data = self.get_zone_data();
        let result = owned_zone_data
            .into_iter()
            .zip(self.previous_data.clone().into_iter())
            .filter(|(current, previous)| current != previous)
            .map(|(current, _)| current)
            .collect::<Vec<ZoneData>>();
        self.previous_data = self.get_zone_data();
        result
    }

Well at first you had two issues one is that you are owning get_zone_data() but trying to return a reference to a structure that holds a reference to it which wouldn't work, because once the owned value goes out of the scope its gonna be destroyed.
so basically like ping-pong you should own the data that was created in your scope and return it fully(not just a reference to it), or you can let a struct of a wider scope own it and use it.

5

my brain keeps telling me that std::mem::replace should be unsafe idk why

6

I had a similar reaction when learning Rust. I think it was mainly because my other exposures to std::mem were transmute (definitely unsafe) and forget (which I thought may help me work around borrow check errors, before I understood them). I.e. "doing something questionable" tools.

Now I know std::mem::{replace,swap,take} are all safe and useful functions which are just underrepresented in learning material for some reason.

7 Likes
7

Thanks all. This is all really helpful, and simple too. I couldn't help feeling that I was stuck in some kind of chicken and the egg situation, and std::mem::replace gets around this nicely. I agree with the comment about std::mem::replace sounding unsafe, and my guess is there is some unsafe under the hood, but will keep it in mind as a tool to manage get around borrowing then moving.

Also, good point about, not necessarily avoiding copy/clone - there is negligible performance loss in explicitly cloning a few small and simple structs and the "expensive" bit is calling get_zone_data().

8

Yes, but it's completely sound -- impossible to be the cause of undefined behavior. There is no reason to not use it.

Quoting:

The biggest failure in Rust‘s communication strategy has been the inability to explain to non-experts that unsafe abstractions are the point, not a sign of failure

E.g. it's impossible to not have the equivalent of unsafe code at the OS interface level, because they're pretty much all C based. But that interface can be encapsulated in a sound abstraction.

(I'm not saying to blindly trust unsafe. In the context of reviewing a PR or dependency, unsafe deserves extra scrutiny, and there's value in being unsafe-free when tenable. But in this case the dependency is the core library which you implicitly trust already. And this particular function is simple to boot.)

3 Likes
9

After seeing your answer yesterday, i started digging deeper into the std::mem:: replace swap and found that there were some attempts to add it to the prelude
Add-std-swap-replace-take-to-the-prelude

1 Like