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

Printf fixes around handling of -0.0 (negative zero) #18147

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Core/9.0.200.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

* Fix exception on Post after MailboxProcessor was disposed ([Issue #17849](https://github.com/dotnet/fsharp/issues/17849), [PR #17922](https://github.com/dotnet/fsharp/pull/17922))
* Fix missing null annotation in Async.SwitchToContext ([Issue #18055](https://github.com/dotnet/fsharp/issues/18055), [PR #18059](https://github.com/dotnet/fsharp/pull/18059))
* Fix printf handling of -0.0 (negative zero) values for float, float32, and decimal values ([Issue #15557](https://github.com/dotnet/fsharp/issues/15557) and [Issue #15558](https://github.com/dotnet/fsharp/issues/15558), [PR #18147](https://github.com/dotnet/fsharp/pull/18147))

### Added

Expand Down
34 changes: 25 additions & 9 deletions src/FSharp.Core/printf.fs
Original file line number Diff line number Diff line change
Expand Up @@ -658,8 +658,12 @@ module internal PrintfImpl =

/// Contains functions to handle left/right and no justification case for numbers
module GenericNumber =

let isPositive (n: obj) =

let inline singleIsPositive n = n >= 0.0f
let inline doubleIsPositive n = n >= 0.0
let inline decimalIsPositive n = n >= 0.0M

let isPositive (n: obj) =
match n with
| :? int8 as n -> n >= 0y
| :? uint8 -> true
Expand All @@ -671,9 +675,9 @@ module internal PrintfImpl =
| :? uint64 -> true
| :? nativeint as n -> n >= 0n
| :? unativeint -> true
| :? single as n -> n >= 0.0f
| :? double as n -> n >= 0.0
| :? decimal as n -> n >= 0.0M
| :? single as n -> singleIsPositive n
| :? double as n -> doubleIsPositive n
| :? decimal as n -> decimalIsPositive n
| _ -> failwith "isPositive: unreachable"

/// handles right justification when pad char = '0'
Expand Down Expand Up @@ -851,12 +855,24 @@ module internal PrintfImpl =
| _ -> invalidArg (nameof spec) "Invalid integer format"

module FloatAndDecimal =


let fixupSign isPositive (nStr: string) =
// .NET Core and .NET Framework differ in how ToString and other formatting methods handle certain negative floating-point values (namely, -0.0 and values which round to -0.0 upon display).
// (see: https://devblogs.microsoft.com/dotnet/floating-point-parsing-and-formatting-improvements-in-net-core-3-0/)
// So in order for F#'s sprintf to behave consistently across platforms, we essentially "polyfill" (normalize) the output to ToString across the two runtimes. Specifically we do this by
// removing the '-' character in situations where the rest of the sprintf logic treats the number as positive, but .NET Core treats it as negative (i.e. -0.0, or -0.0000000001 when
// displaying with only a few decimal places)
// TODO: make this work for numbers like -0.0000000001
if isPositive && nStr.StartsWith "-" then
nStr.Substring 1
else
nStr

let rec toFormattedString fmt (v: obj) =
match v with
| :? single as n -> n.ToString(fmt, CultureInfo.InvariantCulture)
| :? double as n -> n.ToString(fmt, CultureInfo.InvariantCulture)
| :? decimal as n -> n.ToString(fmt, CultureInfo.InvariantCulture)
| :? single as n -> n.ToString(fmt, CultureInfo.InvariantCulture) |> fixupSign (GenericNumber.singleIsPositive n)
| :? double as n -> n.ToString(fmt, CultureInfo.InvariantCulture) |> fixupSign (GenericNumber.doubleIsPositive n)
| :? decimal as n -> n.ToString(fmt, CultureInfo.InvariantCulture) |> fixupSign (GenericNumber.decimalIsPositive n)
| _ -> failwith "toFormattedString: unreachable"

let isNumber (x: obj) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,102 @@ type PrintfTests() =
Assert.AreEqual(" 7B", sprintf "%*X" 8 123 )
Assert.AreEqual("7B ", sprintf "%-*X" 8 123 )

// test cases for https://github.com/dotnet/fsharp/issues/15557
[<Fact>]
member this.``sign flag - positive and negative one``() =
test "%f" +1.0 "1.000000"
test "%f" -1.0 "-1.000000"
test "%+f" +1.0 "+1.000000"
test "%+f" -1.0 "-1.000000"

test "%f" +1.0f "1.000000"
test "%f" -1.0f "-1.000000"
test "%+f" +1.0f "+1.000000"
test "%+f" -1.0f "-1.000000"

test "%f" +1.0M "1.000000"
test "%f" -1.0M "-1.000000"
test "%+f" +1.0M "+1.000000"
test "%+f" -1.0M "-1.000000"

[<Fact>]
member this.``sign flag - positive and negative zero``() =
test "%f" +0.0 "0.000000"
test "%f" -0.0 "0.000000"
test "%+f" +0.0 "+0.000000"
test "%+f" -0.0 "+0.000000"

test "%f" +0.0f "0.000000"
test "%f" -0.0f "0.000000"
test "%+f" +0.0f "+0.000000"
test "%+f" -0.0f "+0.000000"

test "%f" +0.0M "0.000000"
test "%f" -0.0M "0.000000"
test "%+f" +0.0M "+0.000000"
test "%+f" -0.0M "+0.000000"

[<Fact>]
member this.``sign flag - very small positive and negative numbers``() =
test "%f" -0.0000001 "0.000000"
// TODO: should this output -0.000000 or +0.000000? See https://github.com/dotnet/fsharp/pull/18147#issuecomment-2546220183
// test "%+f" -0.0000001 "+0.000000"

test "%f" -0.0000001f "0.000000"
// see previous comment
// test "%+f" -0.0000001f "+0.000000"

test "%f" -0.0000001M "0.000000"
// see previous comment
// test "%+f" -0.0000001M "+0.000000"

[<Fact>]
member this.``sign flag - infinity``() =
test "%f" +infinity "Infinity"
test "%f" -infinity "-Infinity"
test "%+f" +infinity "Infinity"
test "%+f" -infinity "-Infinity"

test "%f" +infinityf "Infinity"
test "%f" -infinityf "-Infinity"
test "%+f" +infinityf "Infinity"
test "%+f" -infinityf "-Infinity"

[<Fact>]
member this.``sign flag - NaN``() =
test "%f" +nan "NaN"
test "%f" -nan "NaN"
test "%+f" +nan "NaN"
test "%+f" -nan "NaN"

test "%f" +nanf "NaN"
test "%f" -nanf "NaN"
test "%+f" +nanf "NaN"
test "%+f" -nanf "NaN"

// test cases for https://github.com/dotnet/fsharp/issues/15558 (same root cause as #15557; listing for completeness)
[<Fact>]
member this.``zero padding - positive and negative one`` () =
test "%010.3f" +1.0 "000001.000"
test "%010.3f" -1.0 "-00001.000"

test "%010.3f" +1.0f "000001.000"
test "%010.3f" -1.0f "-00001.000"

test "%010.3f" +1.0M "000001.000"
test "%010.3f" -1.0M "-00001.000"

[<Fact>]
member this.``zero padding - positive and negative zero`` () =
test "%010.3f" +0.0 "000000.000"
test "%010.3f" -0.0 "000000.000"

test "%010.3f" +0.0f "000000.000"
test "%010.3f" -0.0f "000000.000"

test "%010.3f" +0.0M "000000.000"
test "%010.3f" -0.0M "000000.000"

[<Fact>]
member _.``union case formatting`` () =
Assert.AreEqual("CaseOne", sprintf "%A" CaseOne)
Expand Down
Loading