about summary refs log tree commit diff
path: root/corp
diff options
context:
space:
mode:
authorFlorian Klink <flokli@flokli.de>2023-01-24T18·27+0100
committerclbot <clbot@tvl.fyi>2023-01-25T07·49+0000
commit1facd889bba724cf20ea14422ee1e57440b3e761 (patch)
tree114d353331e7387bdb9955f767e5982eeb4fb9ca /corp
parent192dac5a749edece1b5b3fb0b8acb92819df22e0 (diff)
feat(tvix/eval): use lexical-core to format float r/5753
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 <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Diffstat (limited to 'corp')
-rw-r--r--corp/tvixbolt/Cargo.lock72
1 files changed, 72 insertions, 0 deletions
diff --git a/corp/tvixbolt/Cargo.lock b/corp/tvixbolt/Cargo.lock
index 2bfdc1b344..033335e84a 100644
--- a/corp/tvixbolt/Cargo.lock
+++ b/corp/tvixbolt/Cargo.lock
@@ -334,6 +334,70 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646"
 
 [[package]]
+name = "lexical-core"
+version = "0.8.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "2cde5de06e8d4c2faabc400238f9ae1c74d5412d03a7bd067645ccbc47070e46"
+dependencies = [
+ "lexical-parse-float",
+ "lexical-parse-integer",
+ "lexical-util",
+ "lexical-write-float",
+ "lexical-write-integer",
+]
+
+[[package]]
+name = "lexical-parse-float"
+version = "0.8.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "683b3a5ebd0130b8fb52ba0bdc718cc56815b6a097e28ae5a6997d0ad17dc05f"
+dependencies = [
+ "lexical-parse-integer",
+ "lexical-util",
+ "static_assertions",
+]
+
+[[package]]
+name = "lexical-parse-integer"
+version = "0.8.6"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "6d0994485ed0c312f6d965766754ea177d07f9c00c9b82a5ee62ed5b47945ee9"
+dependencies = [
+ "lexical-util",
+ "static_assertions",
+]
+
+[[package]]
+name = "lexical-util"
+version = "0.8.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "5255b9ff16ff898710eb9eb63cb39248ea8a5bb036bea8085b1a767ff6c4e3fc"
+dependencies = [
+ "static_assertions",
+]
+
+[[package]]
+name = "lexical-write-float"
+version = "0.8.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "accabaa1c4581f05a3923d1b4cfd124c329352288b7b9da09e766b0668116862"
+dependencies = [
+ "lexical-util",
+ "lexical-write-integer",
+ "static_assertions",
+]
+
+[[package]]
+name = "lexical-write-integer"
+version = "0.8.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e1b6f3d1f4422866b68192d62f77bc5c700bee84f3069f2469d7bc8c77852446"
+dependencies = [
+ "lexical-util",
+ "static_assertions",
+]
+
+[[package]]
 name = "libc"
 version = "0.2.134"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -595,6 +659,12 @@ dependencies = [
 ]
 
 [[package]]
+name = "static_assertions"
+version = "1.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f"
+
+[[package]]
 name = "syn"
 version = "1.0.101"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -657,6 +727,8 @@ dependencies = [
  "codemap-diagnostic",
  "dirs",
  "imbl",
+ "lazy_static",
+ "lexical-core",
  "path-clean",
  "regex",
  "rnix",