Why does `move` not appear to move ownership here?

help
1

My expectation is that the closure func takes ownership of the struct instance x and that the program would not compile because there is an attempt to mutate x after its ownership was moved to the closure.

Instead the program compiles fine and provides the output

Found x.0 = 16

Can someone explain why this work?

struct MyType {    
  member: u32,
}

fn main() {

  let mut x = MyType {member: 15};
  let mut func = move || {
        x.member +=1;
  };
  
  func();

  x.member += 1;

  // This prints 
  println!("Found x.0 = {}", x.member);

}
2

This is Rust trying to get smart and move only a single used field, not the whole struct (and since it's Copy, struct is not invalidated). You can see that if field is not Copy, for example String, this is an error:

struct MyType {    
  member: String,
}

fn main() {

  let mut x = MyType {member: "a".into()};
  let mut func = move || {
        x.member += "b";
  };
  
  func();

  x.member += "c";

  println!("Found x.0 = {}", x.member);

}

And if you really want to move the whole struct for some reason, you can just add let [mut] x = x; to the beginning of the closure:

struct MyType {    
  member: u32,
}

fn main() {

  let mut x = MyType {member: 15};
  let mut func = move || {
        let mut x = x;
        x.member +=1;
  };
  
  func();

  x.member += 1;

  println!("Found x.0 = {}", x.member);

}
7 Likes
3

I understand, but it seems very wrong to me. Any reasonable interpretation of this code (by a human) would be that x.member += 1 is meant to refer to the field in the struct. Instead it results in an unexpected bug.

Is this really considered correct and desirable? Or was it something accidental and now has to be supported for compatibility?

4

It was intentional and is edition-dependent (the old closure capture behavior is still present on pre-2021 editions).

2 Likes
5

It's also reasonable to think that this code should compile:

fn main() {
    let pair = (String::from(""), String::from(""));
    
    let f1 = move || pair.0;
    let f2 = move || pair.1;
    
    f1();
    f2();
}

Yet before the discussed change it would not compile, since f1 would consume the whole pair.

6 Likes
6

Yes, that's very nice, I agree. It's allowing it when the struct is referenced after calling the closure, and closure mutates the struct, that strikes me as dangerous.

7

I don't think I understand what you meant here. Can you give an example to illustrate what's exactly dangerous?

8

I meant the bug in the OP code that prints 16 instead of 17.

9

I thought it was too pedantic to point this out before, but since it's being directly talked about now -- I'm not sure I'd call the specific example at hand a bug. The disconnect for the OP is that x wasn't moved, like it is on edition 2018 and before. But when that's the case, the program doesn't compile at all. Can a program that only newly compiles be bugged?

I can see the potential for a "16 vs 17" bug though. In that case the bug was putting move on the closure when you wanted to capture by &mut _. But it is also probably something pretty hard to correctly lint against: both the behavior with and without move on the closure are completely valid programs that do different things. Programmer intent is hard to lint, and the suggested change can result in different behavior, so false negatives would be bad.

The surprise (to some) of move not actually moving Copy values exists on all editions (although it's true that it's significantly less likely to hit when move captures the whole containing value, pre-2021). Maybe there could a be a restriction lint that just lets you know when you've copied something into a move closure.

7 Likes
10

Great idea.

11

It looks like this doc is outdated:

https://doc.rust-lang.org/1.82.0/reference/types/closure.html#capture-modes

Composite types such as structs, tuples, and enums are always captured entirely, not by individual fields. It may be necessary to borrow into a local variable in order to capture a single field:

The example given does not use move, but the statement above is general so it seems to include move captures.

Also the example given:

struct SetVec {
    set: HashSet<u32>,
    vec: Vec<u32>
}

impl SetVec {
    fn populate(&mut self) {
        let vec = &mut self.vec;
        self.set.iter().for_each(|&n| {
            vec.push(n);
        })
    }
}

If, instead, the closure were to use self.vec directly, then it would attempt to capture self by mutable reference. But since self.set is already borrowed to iterate over, the code would not compile.

does compile when self.vec is used directly.

I couldn't find an issue about the outdated doc for the reference. Should I file an issue?

I also didn't see anything mentioned about this feature in the book.

12

Looks like a PR got merged less than 24 hours ago. I didn't review it.

2 Likes
13

I think the lint I'd prefer to trigger here is unused_assignments, which would trigger in the case of top-level variables but doesn't currently for struct members. That would clue you in that the closure doesn't do anything.

2 Likes
14

Wow! I should have searched PRs.

15

I agree. I think that would directly address this.

16

I found a couple of related tests, but don't know the explanation of the comments within.

    let mut sum = MyStruct{ a: 1, b: 0};
    (1..10).for_each(move |x| {
        // This will not trigger a warning for unused variable
        // as sum.b will be treated as a Non-tracked place
        sum.b += x;

But it may start warning in the future?

1 Like