What is the right way to handle move data error?

Following code throws following error.

    pub fn modify_date(& mut self, date: &RtcDate) {

        self.ticks = 0_rtc::get_rtc_counter();

        self.date.year = date.year;
        self.date.month = date.month;
        self.date.week = date.week;
        self.date.day = date.day;
    }

self.date.month = date.month;
| ^^^^^^^^^^ move occurs because date.month has type RtcMonths, which does not implement the Copy trait

As I googled, its seems to #[derive (copy, clone)] would make compiler to make a copy. In low level HAL, is this the right way? or since the data is just read any better way? Also, if copy is derived, then every time this function is called, will new copies be created?

Hi!

It depends on what kind of data you are working with, but you don’t give much context to understand your situation. You mention HAL. Are you writing an embedded software? Are you using some sort of library? I’ll give a general answer.

Is date.year something cheap to copy? In that case, derive Clone + Copy on its type and you’re done.

Otherwise, you probably need to take ownership of date by moving it:

    pub fn modify_date(&mut self, date: RtcDate) { // <- notice the absence of `&`
        self.ticks = 0_rtc::get_rtc_counter();

        self.date.year = date.year;
        self.date.month = date.month;
        self.date.week = date.week;
        self.date.day = date.day;
    }

Maybe you can even do something like this:

    pub fn modify_date(&mut self, date: RtcDate) {
        self.ticks = 0_rtc::get_rtc_counter();
        self.date = date;
    }

Also, if copy is derived, then every time this function is called, will new copies be created?

The data will be copied, but be aware that moving is not preventing all copies either. There is no difference between moving a u8 and copying a u8. However, moving a Vec<u8> is very different. In this case, the pointer is copied, but all the data is not and remains at the same place on the heap.
If you are working with trivial types such as newtype wrappers around u8, u32, etc, it’s generally okay to implement Copy, but you may want to not do that deliberately if you need the move semantic for API design purposes (e.g.: ensuring a value is used at most once).

1 Like
pub enum RtcMonths{
    January,
    February,
    March,
    April,
    May,
    June,
    July,
    August,
    September,
    October,
    November,
    December,
}

Thanks and above enum was missing and here is the background on this code: its a embedded (no-std) and to built as library to be called from application not controlled or developed by me.
In this case, my thought is if the parameter is consumed by this function, then if caller can't reuse the date again.

So decided to pass as reference and yes its a mistake that & reference was missing. Does copy mean to copy from src to dest ? or create a duplicate copy?
Further, though the function copies all, it throws error only at the month.

pub fn modify_date(& mut self, date: &RtcDate) {

    self.ticks = fe310_rtc::get_rtc_counter();

    self.date.year = date.year;
    self.date.month = date.month; => error is only here. 
    self.date.week = date.week;
    self.date.day = date.day;

}

I see!

Here, it’s perfectly fine to derive Clone and Copy on your RtcMonths enum, because this is a field-less enum (which also happens to be unit-only enum here).

#[derive(Clone, Copy)]
pub enum RtcMonths {
    …
}

Copying such enums is trivial and even more efficient than passing a reference to them in most cases (though it does not matter in this specific case since you pass the whole RtcDate by reference).

I’m sorry, this sounds like the same thing to me. Copy means that the value can be copied from some source into some destination, and the value is effectively duplicated at the destination. From now on, modifying the source does not affect the destination and vice versa. Furthermore, Copy is a marker trait telling the compiler that the value is trivial to duplicate by copying bits, but unless custom logic is implemented, Clone is doing the same thing. It’s customary to not implement Copy when duplicating the value is considered expensive so that it’s harder for the user to write code with hidden expensive copies. (The official documentation for Copy is an excellent read for that matter.)

Let’s elaborate a bit more for your specific situation. Assuming RtcMonths is defined as:

#[derive(Clone)]
pub struct RtcDate {
    year: u16,
    month: RtcMonths,
    week: u8,
    day: u8,
}

Then the most efficient is:

pub fn set_date_by_move(&mut self, date: RtcDate) {
    self.date = date;
}

ASM:

example::MyHolder::set_date_by_move:
        mov     dword ptr [rdi], esi
        shr     rsi, 32
        mov     word ptr [rdi + 4], si
        ret

Followed closely by:

pub fn set_date_ref_1(&mut self, date: &RtcDate) {
    self.date = date.clone();
}

ASM:

example::MyHolder::set_date_by_ref_1:
        mov     eax, dword ptr [rsi]
        movzx   ecx, byte ptr [rsi + 4]
        mov     dword ptr [rdi], eax
        mov     byte ptr [rdi + 4], cl
        ret

With that one being actually the least efficient:

pub fn set_date_by_ref_2(&mut self, date: &RtcDate) {
    self.date.year = date.year;
    self.date.month = date.month;
    self.date.week = date.week;
    self.date.day = date.day;
}

ASM:

example::MyHolder::set_date_by_ref_2:
        movzx   eax, word ptr [rsi]
        mov     word ptr [rdi], ax
        movzx   eax, byte ptr [rsi + 4]
        mov     byte ptr [rdi + 4], al
        movzx   eax, byte ptr [rsi + 2]
        mov     byte ptr [rdi + 2], al
        movzx   eax, byte ptr [rsi + 3]
        mov     byte ptr [rdi + 3], al
        ret

But if you enable cross-crate inlining by adding the #[inline] attribute on the function, then it’s all optimized into the exact same machine code:

example::user_code_by_move:
        mov     dword ptr [rsp - 16], 470026215
        mov     byte ptr [rsp - 12], 8
        lea     rax, [rsp - 16]
        mov     qword ptr [rsp - 8], rax
        lea     rax, [rsp - 8]
        mov     word ptr [rsp - 12], 8
        mov     dword ptr [rsp - 16], 470026215
        mov     word ptr [rsp - 4], 8
        mov     dword ptr [rsp - 8], 470026215
        mov     dword ptr [rsp - 8], 470026215
        mov     byte ptr [rsp - 4], 8
        ret

example::user_code_by_ref_1:
        mov     dword ptr [rsp - 16], 470026215
        mov     byte ptr [rsp - 12], 8
        lea     rax, [rsp - 16]
        mov     qword ptr [rsp - 8], rax
        lea     rax, [rsp - 8]
        mov     dword ptr [rsp - 16], 470026215
        mov     byte ptr [rsp - 12], 8
        movzx   ecx, word ptr [rsp - 12]
        mov     word ptr [rsp - 4], cx
        mov     dword ptr [rsp - 8], 470026215
        mov     dword ptr [rsp - 8], 470026215
        mov     byte ptr [rsp - 4], 8
        ret

example::user_code_by_ref_2:
        mov     dword ptr [rsp - 16], 470026215
        mov     byte ptr [rsp - 12], 8
        lea     rax, [rsp - 16]
        mov     qword ptr [rsp - 8], rax
        lea     rax, [rsp - 8]
        mov     dword ptr [rsp - 16], 470026215
        mov     byte ptr [rsp - 12], 8
        movzx   ecx, word ptr [rsp - 12]
        mov     word ptr [rsp - 4], cx
        mov     dword ptr [rsp - 8], 470026215
        mov     dword ptr [rsp - 8], 470026215
        mov     byte ptr [rsp - 4], 8
        ret

See the result in Compiler Explorer on godbolt.org.
Here is the code I wrote for Compiler Explorer in case the link dies:

#[derive(Clone, Copy)]
pub enum Month {
    January,
    February,
    March,
    April,
    May,
    June,
    July,
    August,
    September,
    October,
    November,
    December,
}

#[derive(Clone)]
pub struct Date {
    pub year: u16,
    pub month: Month,
    pub week: u8,
    pub day: u8,
}

pub struct MyHolder {
    date: Date,
}

impl MyHolder {
    pub fn set_date_by_move(&mut self, date: Date) {
        self.date = date;
    }

    pub fn set_date_by_ref_1(&mut self, date: &Date) {
        self.date = date.clone();
    }

    pub fn set_date_by_ref_2(&mut self, date: &Date) {
        self.date.year = date.year;
        self.date.month = date.month;
        self.date.week = date.week;
        self.date.day = date.day;
    }
}

pub fn user_code_by_move() {
    let mut holder = MyHolder {
        date: Date {
            year: 2023,
            month: Month::September,
            week: 4,
            day: 28,
        },
    };

    // Do stuff with holder
    std::hint::black_box(&mut holder);

    let new_date = Date {
        year: 2023,
        month: Month::September,
        week: 4,
        day: 28,
    };

    holder.set_date_by_move(new_date.clone());

    // Do stuff with holder
    std::hint::black_box(holder);

    // I can still use new_date since I moved a clone above
    std::hint::black_box(new_date);
}

pub fn user_code_by_ref_1() {
    let mut holder = MyHolder {
        date: Date {
            year: 2023,
            month: Month::September,
            week: 4,
            day: 28,
        },
    };

    // Do stuff with holder
    std::hint::black_box(&mut holder);

    let new_date = Date {
        year: 2023,
        month: Month::September,
        week: 4,
        day: 28,
    };

    holder.set_date_by_ref_1(&new_date);

    // Do stuff with holder
    std::hint::black_box(holder);

    // I can still use new_date since I didn’t move it above
    std::hint::black_box(new_date);
}

pub fn user_code_by_ref_2() {
    let mut holder = MyHolder {
        date: Date {
            year: 2023,
            month: Month::September,
            week: 4,
            day: 28,
        },
    };

    // Do stuff with holder
    std::hint::black_box(&mut holder);

    let new_date = Date {
        year: 2023,
        month: Month::September,
        week: 4,
        day: 28,
    };

    holder.set_date_by_ref_2(&new_date);

    // Do stuff with holder
    std::hint::black_box(holder);

    // I can still use new_date since I didn’t move it above
    std::hint::black_box(new_date);
}

Note that I didn’t put the #[inline] attribute here because it’s all in the same crate, and inlining will happen regardless of the annotation in this case. (Note that #[inline] is different from #[inline(always)], and you should generally not use #[inline(always)] without proper benchmarking.)

So it’s up to you to choose what fits your API the best, but I would recommend either set_date_by_move or set_date_by_ref_1. Both are more maintainable and offer better optimization opportunities to the compiler. set_date_by_ref_1 might be more ergonomic for the user in some cases (you basically hide the .clone() call).

I hope it helps!

3 Likes

Just to give extra emphasis to this: whenever you have an enum that you're using like a C or Java enum, you should #[derive(Copy, Clone, PartialEq, Eq, Hash)] on it.

It'll make your life way easier, and it'll probably even be faster to boot.

1 Like

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.