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

Overflow in division #117

Closed
sharkdp opened this issue Jul 23, 2023 · 2 comments
Closed

Overflow in division #117

sharkdp opened this issue Jul 23, 2023 · 2 comments

Comments

@sharkdp
Copy link

sharkdp commented Jul 23, 2023

The following program

use num_rational::Rational64;

fn main() {
    let a = Rational64::new(1, 3000000000);
    let b = Rational64::from_integer(3080000000);
    println!("{}", a / b);
}

panics with

thread 'main' panicked at 'attempt to multiply with overflow', /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/ops/arith.rs:352:1
stack backtrace:
   0: rust_begin_unwind
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/panicking.rs:578:5
   1: core::panicking::panic_fmt
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/panicking.rs:67:14
   2: core::panicking::panic
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/panicking.rs:117:5
   3: <i64 as core::ops::arith::Mul>::mul
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/ops/arith.rs:345:45
   4: <num_rational::Ratio<T> as core::ops::arith::Div>::div
             at /home/shark/.cargo/registry/src/index.crates.io-6f17d22bba15001f/num-rational-0.4.1/src/lib.rs:738:13
   5: t::main
             at ./src/main.rs:6:20
   6: core::ops::function::FnOnce::call_once
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/ops/function.rs:250:5

in this line:

self.denom / gcd_bd * (rhs.numer / gcd_ac),

Both large integers are far from i64::MAX — but their product overflows the 64-bit boundary. Is the panic expected in this case?

This looks similar to #6 because it also happens in division, but I think it's a separate issue.

@cuviper
Copy link
Member

cuviper commented Jul 24, 2023

Both large integers are far from i64::MAX — but their product overflows the 64-bit boundary. Is the panic expected in this case?

Yes, this is expected. The correct fully-reduced result would be "1 / 9223372036854775808", and that denominator is too large for i64. You can use CheckedDiv to guard against the panic though.

@sharkdp
Copy link
Author

sharkdp commented Jul 24, 2023

Thank you for the explanation. I didn't think about using .checked_div — that works perfectly!

@sharkdp sharkdp closed this as completed Jul 24, 2023
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

No branches or pull requests

2 participants