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

Implement Checked{Add,Sub,Mul,Div} #11

Merged
merged 2 commits into from
Jan 19, 2018
Merged

Implement Checked{Add,Sub,Mul,Div} #11

merged 2 commits into from
Jan 19, 2018

Conversation

psimonyi
Copy link

When the T of a Ratio<T> supports checked arithmetic, the ratio should too.

I'm not really sure whether the checked_* functions should be #[inline] or not. They are in this patch because the non-checked operations are and the checked ones don't look that much bigger.

Peter Simonyi added 2 commits December 29, 2017 01:01
Commit 42aef66 used ? on Option values which is only supported since
Rust 1.22, so replace it with a macro otry! that's essentially the same
as std::try! but for Option instead of Result.
@psimonyi
Copy link
Author

This is needed to support stepped iterators over Ratios:

extern crate num;

fn main() {
    let step = num::rational::Ratio::new(1, 3);
    for x in num::range_step(0.into(), 2.into(), step) {
        println!("{}", x);
    }
}

@psimonyi
Copy link
Author

@cuviper ?

@cuviper
Copy link
Member

cuviper commented Jan 19, 2018

Sorry for neglecting this -- it looks good!

bors r+

More generally, there's a lot of room to improve overflows. For example 1/128 + 1/128 overflows in u8, but if the denominator used LCM it could compute 2/128 -> 1/64. This is a problem for both add and checked_add here, so ideally we'd fix both -- only difference being in panic or None. But anyway, this is stuff for future work. I suppose I should file an issue.

bors bot added a commit that referenced this pull request Jan 19, 2018
11: Implement Checked{Add,Sub,Mul,Div} r=cuviper a=psimonyi

When the `T` of a `Ratio<T>` supports checked arithmetic, the ratio should too.

I'm not really sure whether the checked_* functions should be `#[inline]` or not.  They are in this patch because the non-checked operations are and the checked ones don't look that much bigger.
@bors
Copy link
Contributor

bors bot commented Jan 19, 2018

Build succeeded

@bors bors bot merged commit 13e1678 into rust-num:master Jan 19, 2018
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