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 string interpolation for floats #39

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions starlark/src/eval/tests/go.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,6 @@ fn test_go() {
"int(1e100)",
"1000000 * 1000000 * 1000000",
"int overflow in starlark-rust",
// str for floats doesn't conform to the spec
"assert.eq(str(1.23e45),",
"assert.eq(str(-1.23e-45),",
"assert.eq(str(sorted([inf, neginf, nan, 1e300, -1e300,",
// string interpolation for floats not implemented
"%d",
"%e",
"%f",
"%g",
],
));
assert.conformance(&ignore_bad_lines(
Expand Down
34 changes: 32 additions & 2 deletions starlark/src/values/interpolation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use anyhow::anyhow;
use gazebo::{cast, prelude::*};
use thiserror::Error;

use crate::values::{dict::Dict, tuple::Tuple, Value, ValueError, ValueLike};
use crate::values::{Value, ValueError, ValueLike, dict::Dict, float, num, tuple::Tuple};

/// Operator `%` format or evaluation errors
#[derive(Clone, Dupe, Debug, Error)]
Expand Down Expand Up @@ -75,7 +75,17 @@ pub(crate) fn percent(format: &str, value: Value) -> anyhow::Result<String> {
}
}
b'r' => next_value()?.collect_repr(out),
b'd' => write!(out, "{}", next_value()?.to_int()?).unwrap(),
b'd' => {
let value = next_value()?;
if let Some(num::Num::Float(v)) = value.unpack_num() {
match num::Num::Float(v.trunc()).as_int() {
None => return ValueError::unsupported(&float::StarlarkFloat(v), "%d"),
Some(v) => write!(out, "{}", v).unwrap(),
}
} else {
write!(out, "{}", value.to_int()?).unwrap()
}
}
b'o' => {
let v = next_value()?.to_int()?;
write!(
Expand Down Expand Up @@ -106,6 +116,26 @@ pub(crate) fn percent(format: &str, value: Value) -> anyhow::Result<String> {
)
.unwrap()
}
b'e' => {
let v = next_value()?.unpack_num().ok_or(ValueError::IncorrectParameterType)?.as_float();
float::write_scientific(out, v, 'e', false).unwrap()
}
b'E' => {
let v = next_value()?.unpack_num().ok_or(ValueError::IncorrectParameterType)?.as_float();
float::write_scientific(out, v, 'E', false).unwrap()
}
b'f' | b'F' => {
let v = next_value()?.unpack_num().ok_or(ValueError::IncorrectParameterType)?.as_float();
float::write_decimal(out, v).unwrap()
}
b'g' => {
let v = next_value()?.unpack_num().ok_or(ValueError::IncorrectParameterType)?.as_float();
float::write_compact(out, v, 'e').unwrap()
}
b'G' => {
let v = next_value()?.unpack_num().ok_or(ValueError::IncorrectParameterType)?.as_float();
float::write_compact(out, v, 'E').unwrap()
}
c => {
res.push(b'%');
res.push(c);
Expand Down
173 changes: 160 additions & 13 deletions starlark/src/values/types/float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,88 @@ use crate::values::{
StarlarkValue, Value, ValueError,
};

const WRITE_PRECISION: usize = 6;

fn write_non_finite<W: fmt::Write>(output: &mut W, f: f64) -> fmt::Result {
debug_assert!(f.is_nan() || f.is_infinite());
if f.is_nan() {
write!(output, "nan")
} else {
write!(output, "{}inf", if f.is_sign_positive() { "+" } else { "-" })
}
}

pub fn write_decimal<W: fmt::Write>(output: &mut W, f: f64) -> fmt::Result {
if !f.is_finite() {
write_non_finite(output, f)
} else {
write!(output, "{:.prec$}", f, prec = WRITE_PRECISION)
}
}

pub fn write_scientific<W: fmt::Write>(output: &mut W, f: f64, exponent_char: char, strip_trailing_zeros: bool) -> fmt::Result {
if !f.is_finite() {
write_non_finite(output, f)
} else {
let abs = f.abs();
let exponent = if f == 0.0 { 0 } else { abs.log10().floor() as i32 };
let normal = if f == 0.0 { 0.0 } else { abs / 10f64.powf(exponent as f64) };

// start with "-" for a negative number
if f.is_sign_negative() {
output.write_char('-')?
}

// use the whole integral part of normal (a single digit)
output.write_fmt(format_args!("{}", normal.trunc()))?;

// calculate the fractional tail for given precision
let mut tail = (normal.fract() * 10f64.powf(WRITE_PRECISION as f64)).round() as u64;
let mut rev_tail = Vec::with_capacity(WRITE_PRECISION);
let mut removing_trailing_zeros = strip_trailing_zeros;
for _ in 0..WRITE_PRECISION {
let tail_digit = tail % 10;
if tail_digit != 0 || !removing_trailing_zeros {
removing_trailing_zeros = false;
rev_tail.push(tail_digit as u8);
}
tail /= 10;
}

// write fractional part
if !rev_tail.is_empty() {
output.write_char('.')?;
}
for digit in rev_tail.into_iter().rev() {
output.write_char((b'0' + digit) as char)?;
}

// add exponent part
output.write_char(exponent_char)?;
output.write_fmt(format_args!("{:+03}", exponent))
}
}

pub fn write_compact<W: fmt::Write>(output: &mut W, f: f64, exponent_char: char) -> fmt::Result {
if !f.is_finite() {
write_non_finite(output, f)
} else {
let abs = f.abs();
let exponent = if f == 0.0 { 0 } else {abs.log10().floor() as i32 };

if exponent.abs() >= WRITE_PRECISION as i32 {
// use scientific notation if exponent is outside of our precision (but strip 0s)
write_scientific(output, f, exponent_char, true)
} else if f.fract() == 0.0 {
// make sure there's a fractional part even if the number doesn't have it
output.write_fmt(format_args!("{:.1}", f))
} else {
// rely on the built-in formatting otherwise
output.write_fmt(format_args!("{}", f))
}
}
}

#[derive(Clone, Dupe, Copy, Debug, AnyLifetime)]
pub struct StarlarkFloat(pub f64);

Expand Down Expand Up @@ -70,19 +152,7 @@ where

impl Display for StarlarkFloat {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if self.0.is_nan() {
write!(f, "nan")
} else if self.0.is_infinite() {
if self.0.is_sign_positive() {
write!(f, "+inf")
} else {
write!(f, "-inf")
}
} else if self.0.fract() == 0.0 {
write!(f, "{:.1}", self.0)
} else {
write!(f, "{}", self.0)
}
write_compact(f, self.0, 'e')
}
}

Expand Down Expand Up @@ -209,6 +279,83 @@ impl<'v> StarlarkValue<'v> for StarlarkFloat {
#[cfg(test)]
mod tests {
use crate::assert;
use super::*;

fn non_finite(f: f64) -> String {
let mut buf = String::new();
write_non_finite(&mut buf, f).unwrap();
buf
}

#[test]
fn test_write_non_finite() {
assert_eq!(non_finite(f64::NAN), "nan");
assert_eq!(non_finite(f64::INFINITY), "+inf");
assert_eq!(non_finite(f64::NEG_INFINITY), "-inf");
}

#[test]
#[should_panic]
fn test_write_non_finite_only_for_non_finite() {
non_finite(0f64);
}

fn decimal(f: f64) -> String {
let mut buf = String::new();
write_decimal(&mut buf, f).unwrap();
buf
}

#[test]
fn test_write_decimal() {
assert_eq!(decimal(f64::NAN), "nan");
assert_eq!(decimal(f64::INFINITY), "+inf");
assert_eq!(decimal(f64::NEG_INFINITY), "-inf");

assert_eq!(decimal(0f64), "0.000000");
assert_eq!(decimal(std::f64::consts::PI), "3.141593");
assert_eq!(decimal(-std::f64::consts::E), "-2.718282");
assert_eq!(decimal(1e10), "10000000000.000000");
}

fn scientific(f: f64) -> String {
let mut buf = String::new();
write_scientific(&mut buf, f, 'e', false).unwrap();
buf
}

#[test]
fn test_write_scientific() {
assert_eq!(scientific(f64::NAN), "nan");
assert_eq!(scientific(f64::INFINITY), "+inf");
assert_eq!(scientific(f64::NEG_INFINITY), "-inf");

assert_eq!(scientific(0f64), "0.000000e+00");
assert_eq!(scientific(1.23e45), "1.230000e+45");
assert_eq!(scientific(-3.14e-145), "-3.140000e-145");
assert_eq!(scientific(1e300), "1.000000e+300");
}

fn compact(f: f64) -> String {
let mut buf = String::new();
write_compact(&mut buf, f, 'e').unwrap();
buf
}

#[test]
fn test_write_compact() {
assert_eq!(compact(f64::NAN), "nan");
assert_eq!(compact(f64::INFINITY), "+inf");
assert_eq!(compact(f64::NEG_INFINITY), "-inf");

assert_eq!(compact(0f64), "0.0");
assert_eq!(compact(std::f64::consts::PI), "3.141592653589793");
assert_eq!(compact(-std::f64::consts::E), "-2.718281828459045");
assert_eq!(compact(1e10), "1e+10");
assert_eq!(compact(1.23e45), "1.23e+45");
assert_eq!(compact(-3.14e-145), "-3.14e-145");
assert_eq!(compact(1e300), "1e+300");
}

#[test]
fn test_arithmetic_operators() {
Expand Down
4 changes: 3 additions & 1 deletion starlark/testcases/eval/go/float.star
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,9 @@ assert.true(not (nan < nan))
assert.true(not (nan != nan)) # unlike Python
# Sort is stable: 0.0 and -0.0 are equal, but they are not permuted.
# Similarly 1 and 1.0.
assert.eq(str(sorted([inf, neginf, nan, 1e300, -1e300, 1.0, -1.0, 1, -1, 1e-300, -1e-300, 0, 0.0, negzero, 1e-300, -1e-300])), "[-inf, -1e+300, -1.0, -1, -1e-300, -1e-300, 0, 0.0, -0.0, 1e-300, 1e-300, 1.0, 1, 1e+300, +inf, nan]")
assert.eq(
str(sorted([inf, neginf, nan, 1e300, -1e300, 1.0, -1.0, 1, -1, 1e-300, -1e-300, 0, 0.0, negzero, 1e-300, -1e-300])),
"[-inf, -1e+300, -1.0, -1, -1e-300, -1e-300, 0, 0.0, -0.0, 1e-300, 1e-300, 1.0, 1, 1e+300, +inf, nan]")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restored the original since we no longer ignore this test.


# Sort is stable, and its result contains no adjacent x, y such that y > x.
# Note: Python's reverse sort is unstable; see https://bugs.python.org/issue36095.
Expand Down