Nadrieril / dhall-rust

Maintainable configuration files, for Rust users
Other
303 stars 27 forks source link

Naturals are bounded #200

Closed s-zeng closed 3 years ago

s-zeng commented 3 years ago

I'm not sure if unbounded naturals is within the scope of this project, but this is a bit of behaviour that did bite me when using dhall-rust.

Consider the following program:

use serde_dhall::SimpleValue;

fn main() {
    let my_simple_str = "18446744073709551616";
    let deserialized: SimpleValue = serde_dhall::from_str(my_simple_str).parse().unwrap();
    println!("deserialized = {:?}", deserialized);
}

Running this with cargo run produces a panic, due to surpassing the max size of u64:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error(Dhall(Error { kind: Parse(Error { variant: CustomError { message: "number too large to fit in target type" }, location: Span((0, 20)), line_col: Span((1, 1), (1, 21)), path: None, line: "18446744073709551616", continued_line: None }) }))', src/main.rs:5:37
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

However, the haskell implementation is able to handle this perfectly fine:

$ dhall <<< "18446744073709551616"
18446744073709551616

Since this is a difference in behaviour between the implementations, I figured it should be reported as an issue. Let me know if this is intentional.

Nadrieril commented 3 years ago

It is indeed intentional. Haskell has built-in support for numbers of arbitrary size, but rust and the serde ecosystem don't. It would have been a lot of work for nothing since serde would have rejected it. Do you have a use for such huge numbers?

s-zeng commented 3 years ago

I personally don't, but I found this while writing tests for dhall-python, which is an implementation I maintain that depends on dhall-rust w/ FFI, since Python supports arbitrarily-sized numbers. I guess it's unfeasible to support arbitrarily large numbers while serde doesn't support it. Thanks for the info

basile-henry commented 3 years ago

We could probably add support for deserializing it as a u128, no?

basile-henry commented 3 years ago

Probably not actually, since it's being parsed as a u64 currently: https://github.com/Nadrieril/dhall-rust/blob/8be3891b1e30f61b1f38b96e1ed200f367066032/dhall/src/syntax/ast/expr.rs#L11

Nadrieril commented 3 years ago

I could yeah. But I'd rather wait until someone really needs to store a huge number, to understand what they really need.