Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Copy derives #5

Closed
wants to merge 1 commit into from
Closed

Add Copy derives #5

wants to merge 1 commit into from

Conversation

ketsuban
Copy link

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
Copy link
Member

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.

@oli-obk oli-obk closed this May 16, 2018
@videah videah mentioned this pull request Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants