rust-osdev / volatile

Apache License 2.0
71 stars 19 forks source link

Add Copy derives #5

Closed ketsuban closed 6 years ago

ketsuban commented 6 years ago

I've been improving my VBA buffer logic and found something I can't do.

#[repr(C)]
struct Row([Volatile<Character>; BUFFER_WIDTH]);

impl Row {
    fn blank() -> Row {
        Row(
            [Volatile::new(Character {
                ascii: b' ',
                color: ColorPair::new(Color::Black, Color::Black),
            }); BUFFER_WIDTH],
        )
    }
}

This is inconvenient, and there doesn't appear to be any good reason why Volatile doesn't implement Copy - the contents are required to by trait constraint - so I made a quick pull request.

phil-opp commented 6 years ago

I think we wanted to avoid accidental copies. A volatile is normally bound to a specific memory location (e.g. the VGA buffer or memory mapped registers). You can copy the current value, but copying the volatile itself to a stack variable does not make much sense, since there are no side-effects for stack variables. For example, consider the following code:

// set to true by hardware if button is currently pressed
static BUTTON: &'static mut Volatile<bool> = …;

// correct
while !BUTTON.read() {} // wait until button is pressed

// wrong
let b = *BUTTON;
while !b.read() {} // copy the current button value to the stack and wait until it changes

Yes, the explicit dereference (*BUTTON) makes the copy somewhat visible, but this is not always the case (e.g. automatic dereferencing when it is part of a struct: let b = BUTTON.current_value).

The initialization problem is unfortunate. You could try to use the Default trait, but it only works for fixed array sizes. Maybe Rust will be able to allow initialization with non-Copy types when we get the new constant evaluator. Until then, you can use the array_init crate.