From 1facd889bba724cf20ea14422ee1e57440b3e761 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Tue, 24 Jan 2023 19:27:20 +0100 Subject: feat(tvix/eval): use lexical-core to format float Apparently our naive implementation of float formatting, which simply used {:.5}, and trimmed trailing "0" strings not sufficient. It wrongly trimmed numbers with zeroes but no decimal point, like `10000` got trimmed to `1`. Nix uses `std::to_string` on the double, which according to https://en.cppreference.com/w/cpp/string/basic_string/to_string is equivalent to `std::sprintf(buf, "%f", value)`. https://en.cppreference.com/w/cpp/io/c/fprintf mentions this is treated like this: > Precision specifies the exact number of digits to appear after > the decimal point character. The default precision is 6. In the > alternative implementation decimal point character is written even if > no digits follow it. For infinity and not-a-number conversion style > see notes. This doesn't seem to be the case though, and Nix uses scientific notation in some cases. There's a whole bunch of strategies to determine which is a more compact notation, and which notation should be used for a given number. https://github.com/rust-lang/rust/issues/24556 provides some pointers into various rabbit holes for those interested. This gist seems to be that currently a different formatting is not exposed in rust directly, at least not for public consumption. There is the [lexical-core](https://github.com/Alexhuszagh/rust-lexical) crate though, which provides a way to format floats with various strategies and formats. Change our implementation of `TotalDisplay` for the `Value::Float` case to use that. We still need to do some post-processing, because Nix always adds the sign in scientific notation (and there's no way to configure lexical-core to do that), and lexical-core in some cases keeps the trailing zeros. Even with all that in place, there as a difference in `eval-okay- fromjson.nix` (from tvix-tests), which I couldn't get to work. I updated the fixture to a less problematic number. With this, the testsuite passes again, and does for the upcoming CL introducing builtins.fromTOML, and enabling the nix testsuite bits for it, too. Change-Id: Ie6fba5619e1d9fd7ce669a51594658b029057acc Reviewed-on: https://cl.tvl.fyi/c/depot/+/7922 Tested-by: BuildkiteCI Autosubmit: flokli Reviewed-by: tazjin --- tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.exp | 2 +- tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.nix | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'tvix/eval/src/tests') diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.exp index c855950a30..24aa21d78f 100644 --- a/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.exp +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.exp @@ -1 +1 @@ -[ { Image = { Animated = false; Height = 600; IDs = [ 116 943 234 38793 true false null -100 ]; Latitude = 37.7668; Longitude = -122.3959; Thumbnail = { Height = 125; Url = "http://www.example.com/image/481989943"; Width = 100; }; Title = "View from 15th Floor"; Width = 800; }; } { name = "a"; value = "b"; } [ 1 2 3 4 ] ] +[ { Image = { Animated = false; Height = 600; IDs = [ 116 943 234 38793 true false null -100 ]; Latitude = 37.7668; Longitude = -122.396; Thumbnail = { Height = 125; Url = "http://www.example.com/image/481989943"; Width = 100; }; Title = "View from 15th Floor"; Width = 800; }; } { name = "a"; value = "b"; } [ 1 2 3 4 ] ] diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.nix index e4f6213125..e530789446 100644 --- a/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.nix +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-fromjson.nix @@ -15,7 +15,7 @@ "Animated" : false, "IDs": [116, 943, 234, 38793, true ,false,null, -100], "Latitude": 37.7668, - "Longitude": -122.3959 + "Longitude": -122.396 } } '') -- cgit 1.4.1