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

WIP: Better support for primitive ops #27

Closed
wants to merge 3 commits into from

Conversation

gbrochar
Copy link

See #26 for use case

So this is just the use case I need at the moment and I'm not sure about how it would fit in the library that's why I'm submitting this WIP to know if it's even useful to develop something broader.

It can be easily expanded with

impl<T: core::convert::Into<isize>> core::ops::Add<T> for $name {
    type Output = Self;
    fn add(self, other: T) -> Self {
        Self::new(($inner::from(self) as usize).wrapping_add_signed(other.into()) as $inner)
    }
}

In the restricted_int macro but I need to further investigate this one that's why I didn't commit it.

I have started thinking a bit more about it and it has some implications that's why I wanted to ask you about your views on this, I'm not really used to contributing but I wanna start so apologies if this is inconvenient.

My view is for the specific case of adding signed number to key or velocity it's useful.

I also thought about implementing some ops between u28 and u15 because : Timing::Metrical(u15) and delta: u28. Modulo to do something every n beats is an example use case for this one.

Thank you for your time

@kovaxis
Copy link
Owner

kovaxis commented Jun 15, 2024

Hmmmm I'm not sure I'd like to get into this rabbit hole. For example, + for "normal" types implements checked addition in debug mode and wrapping addition in release mode. However, this PR implements wrapping addition unconditionally. Then there are other operations like -, *, >> and so on.

In fact, for the next breaking release of midly I'd prefer to have the u7-and-family types just be a "transparent" newtype: pub struct u7(pub u8). This would make u7 just a "reminder" that the top bit is useless. The encoder would just ignore that top bit, and the decoder would never set it. Supporting u7 is just too unwieldy both for this library and its users. I'm even considering dropping u7 altogether and using u8 only.

For now, adding two u7 or u14 or whatever will have to go through their corresponding unbounded types, forcing the user to handle overflow manually.

@kovaxis kovaxis closed this Jun 15, 2024
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.

2 participants