Using expression during construction of struct in Rust fails even though the intent is clear

Question

Armed with the knowledge I had gained from answers (Thanks again!) to this question on Stackoverflow, I had written this:

pub struct AggregateDataPerConnection {
    pub connection_idx: u16,
    pub ok_duration_series: Vec<u128>,
    pub mx_time_spent_on_ok_req: u128,

}

// TODO: Provide a Builder for the struct below!
impl AggregateDataPerConnection {

    pub fn new(
        connection_idx: u16, ok_duration_series: Vec<u128>
    ) -> AggregateDataPerConnection {

        AggregateDataPerConnection {
            connection_idx,
            ok_duration_series,
            mx_time_spent_on_ok_req: ok_duration_series.iter().copied().max().unwrap_or(0)

        }
    }
}

The compiler was not happy:

Compiling playground v0.0.1 (/playground)

error[E0382]: borrow of moved value: `ok_duration_series`
  --> src/lib.rs:20:38
   |
12 |         connection_idx: u16, ok_duration_series: Vec<u128>
   |                              ------------------ move occurs because `ok_duration_series` has type `Vec<u128>`, which does not implement the `Copy` trait
...
19 |             ok_duration_series,
   |             ------------------ value moved here
20 |             mx_time_spent_on_ok_req: ok_duration_series.iter().copied().max().unwrap_or(0)
   |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^ value borrowed here after move
   |
   = note: borrow occurs due to deref coercion to `[u128]`

For more information about this error, try `rustc --explain E0382`.

I had reasoned - aided by the compiler - because the move happened before the max was calculated, I could postpone the move, by lexical scoping, like this:

impl AggregateDataPerConnection {

    pub fn new(
        connection_idx: u16, ok_duration_series: Vec<u128>
    ) -> AggregateDataPerConnection {

        AggregateDataPerConnection {
            connection_idx,
            mx_time_spent_on_ok_req: ok_duration_series.iter().copied().max().unwrap_or(0),
            ok_duration_series
        
        }
    }
}

This worked to my relief! :-)

However, I am not confident if my reasoning is correct.

As I am reflecting upon this:

Because the compiler has the complete view of what I intend to do, inside a constructor of the struct, it already knows that there is no way that the control (at the runtime) can seep through (making the Vec vulnerable), during the construction and therefore, the compiler can detect the max w/o having to move the vector. In any case, the movement of the ownership is towards a member of the same struct being constructed.

Please make me wiser.

Playground

`I had reasoned - aided by the compiler - because the move happened before the max was calculated, I could postpone the move, by lexical scoping, like this: [...] However, I am not confident if my reasoning is correct.` This is correct. I didn't really understand your last section though. — tkausl, Mar 12 '23 at 15:18
Thanks for the affirmation. My point is that the compiler knows that I am computing the max of a Vector, before assigning the same Vector to a member variable of the same _struct_ that the max belongs to. So, why can't the compiler loosen the ownership rules for the construction to finish? If it wants, it can rearrange the order. After all the construction is the last expression inside the `new()` function, before returning. @tkausl — Nirmalya, Mar 12 '23 at 17:12
There are multiple cases where the compiler could guess the intent, but for uniformity of the language, to make it easier to learn, and to ease the implementation the language designers choose otherwise. — Chayim Friedman, Mar 12 '23 at 18:05

score 3 · Answer 1 · edited Mar 12 '23 at 17:48

3

Calculate the mx_time_spent_on_ok_req value outside of the AggregateDataPerConnection struct:

pub fn new(
    connection_idx: u16, ok_duration_series: Vec<u128>
) -> AggregateDataPerConnection {

    let mx_time_spent_on_ok_req = ok_duration_series.iter().copied().max().unwrap_or(0);
    AggregateDataPerConnection {
        connection_idx,
        ok_duration_series,
        mx_time_spent_on_ok_req,
    }
}

edited Mar 12 '23 at 17:48

Finomnis

18,094
1
20
27

answered Mar 12 '23 at 17:38

Kaplan

2,572
13
14

1

No? Why would it? The borrow checker isn't concerned with how things are named. – cafce25 Mar 12 '23 at 17:41
1

@Kaplan, I had done that (the variable name was different though), to wiggle out of the problem. The Playground still has the original code, commented! :-) And yes, this is the most clear and readable way to initialize the `max`, and as a bonus, one doesn't face compiler's frown! – Nirmalya Mar 13 '23 at 02:18
And let's not forget the *rustier* [naming practice](https://stackoverflow.com/posts/75715078/revisions) of the `mx_time_spent_on_ok_req` variable by [Finomnis](https://stackoverflow.com/users/2902833/finomnis). – Kaplan Mar 13 '23 at 06:08

score 3 · Answer 2 · answered Mar 12 '23 at 18:08

What @Kaplan says.

If it [(the compiler)] wants, it can rearrange the order

While this might be possible, I think you are too optimistic about this. Yes, in this very simple example this might be true. But in more complex examples, there are exponentially many ways to reorder the code, should the borrow checker try all of them? I don't think including a "would this work if I reordered it" step in the borrow checker would really be feasible.

In any case, the movement of the ownership is towards a member of the same struct being constructed

the compiler can detect the max w/o having to move the vector

I think you give the compiler too much credit here. I don't think the compiler really understands the purpose of a max operation, it's simply an operation as any other operation.

Further, it's important that the order is actually kept. Otherwise, code like this

struct MyValue(i32);

impl MyValue {
    pub fn as_ptr(&self) -> *const i32 {
        &self.0
    }
}

// Yes I know a struct should never store a pointer to one of its own values;
// this is just for demonstration purposes
struct MyStruct {
    pub val: MyValue,
    pub ptr: *const i32,
}

fn main() {
    let val = MyValue(42);

    let my_struct = MyStruct {
        ptr: val.as_ptr(),
        val,
    };

    println!("Array: {:p}", my_struct.val.as_ptr());
    println!("Pointer: {:p}", my_struct.ptr);
}

Array: 0x7fff81c1bcd8
Pointer: 0x7fff81c1bccc

would behave in different ways depending on the compilation. If the compiler reordered things, then it could also print

Array: 0x7fff81c1bcd8
Pointer: 0x7fff81c1bcd8

because it could rearrange the code so that the address is read from the new location.

I know this example is a little on the shady side, but I just wanted to demonstrate that a defined order of things is important. And because that's the case, the other way round won't compile, as the val variable no longer exists after it got moved into the struct.

I'm sure there are better examples, this is just the best I could come up with on the spot :)

The example of `address` being different, in case of a reordering, is an very good point. I didn't think of it. — Nirmalya, Mar 16 '23 at 16:13

score 2 · Accepted Answer · answered Mar 12 '23 at 18:23

The borrow checker doesn't reorder things. Reordering operations is an optimization that would only happen later in compilation, after rustc has determined your program is correct.

The only things the borrow checker knows is that ok_duration_series.iter().copied().max().unwrap_or(0) takes a shared reference to ok_duration_series, while the parameter of the same name takes ownership of ok_duration_series. The former never moves ok_duration_series while the latter always moves it.

There are plenty of things with those properties that would be affected by the order, for example:

let mut max = 0;
AggregateDataPerConnection {
    connection_idx,
    ok_duration_series: {
        ok_duration_series.push(max);
        ok_duration_series
    },
    mx_time_spent_on_ok_req: {
        max = ok_duration_series.iter().copied().max().unwrap_or(0);
        max
    },
}

These both copy max so we don't have to worry about max being moved. However, since the first block requires ownership, this would not compile for the same reason as your first example. If the borrow checker reordered it, it would be confusing since now ok_duration_series gets pushed a value that comes from a later block.

"it would be confusing since now ok_duration_series gets pushed a value that comes from a later block": I agree but the fact that there is a 'later' block, is already known to the compiler. That was my point. — Nirmalya, Mar 13 '23 at 04:04
I am sorry for being late in accepting an answer to my question. All answers here are useful and add to my knowledge for sure. My aim was to understand the rational behind the compiler's decisions and all answers throw sufficient light on that. Thanks, all. I am accepting @drewtato 's answer because the proposition that borrow checker doesn't reorder things (optimization) and `rustc` has to ascertain the correctness first, sounds correct to me, as an answer to my question. — Nirmalya, Mar 16 '23 at 16:12

Using expression during construction of struct in Rust fails even though the intent is clear

3 Answers3