Printf fixes around handling of -0.0 (negative zero)

docs/release-notes/.FSharp.Core/11.0.100.md

@@ -4,6 +4,7 @@
 * Fix `Array.exists2` documentation examples to use equal-length arrays; the previous examples would throw `ArgumentException` at runtime instead of returning the documented `false`/`true` values. ([PR #19672](https://github.com/dotnet/fsharp/pull/19672))
 * Move `Async.StartChild` to the "Starting Async Computations" docs category alongside `Async.StartChildAsTask`. ([Issue #19667](https://github.com/dotnet/fsharp/issues/19667))
 * Add `InlineIfLambda` to `Array.init` ([PR #19869](https://github.com/dotnet/fsharp/pull/19869))
+* 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

src/FSharp.Core/printf.fs

@@ -657,23 +657,19 @@ module internal PrintfImpl =
 
     /// Contains functions to handle left/right and no justification case for numbers
     module GenericNumber =
-
-        let isPositive (n: obj) = 
-            match n with 
-            | :? int8 as n -> n >= 0y
-            | :? uint8 -> true
-            | :? int16 as n -> n >= 0s
-            | :? uint16 -> true
-            | :? int32 as n -> n >= 0
-            | :? uint32 -> true
-            | :? int64 as n -> n >= 0L
-            | :? 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
-            | _ -> failwith "isPositive: unreachable"
+
+        let inline isStrNegative (str: string) = str.Length > 0 && str[0] = '-'
+
+        /// Check if the number is negative -- not with the number itself, but with the formatted ToString representation --
+        /// to ensure that we always agree with the runtime (.NET and .NET Framework differ at -0.0 for floats).
+        /// Single.IsPositive (and similar) would be the right choice, but it is not available in .NET Framework /
+        /// .NET Standard 2.0.
+        /// Additionally: behaviorally speaking, this is really only needed for single and double, at which point it's
+        /// just simpler to uniformly use the same approach for all number types, which also has the added bonus of
+        /// avoiding type tests (micro perf win! https://github.com/dotnet/fsharp/pull/18147#discussion_r3821830744)
+        /// See: https://github.com/dotnet/fsharp/issues/15557
+        /// and: https://github.com/dotnet/fsharp/issues/15558
+        let isPositive (nStr: string) = not (isStrNegative nStr)
 
         /// handles right justification when pad char = '0'
         /// this case can be tricky:
@@ -728,7 +724,9 @@ module internal PrintfImpl =
             if isUnsigned then
                 fun (v: objnull) -> noJustificationCore (f v) true true prefix
             else 
-                fun (v: objnull) -> noJustificationCore (f v) true (isPositive v) prefix
+                fun (v: objnull) ->
+                    let vStr = f v
+                    noJustificationCore vStr true (isPositive vStr) prefix
 
     /// contains functions to handle left/right and no justification case for numbers
     module Integer =
@@ -790,30 +788,35 @@ module internal PrintfImpl =
             else
                 if isGFormat then
                     fun (w: int) (v: objnull) ->
-                        GenericNumber.leftJustifyWithGFormat (f v) true true (GenericNumber.isPositive v) w prefix padChar
+                        let vStr = f v
+                        GenericNumber.leftJustifyWithGFormat vStr true true (GenericNumber.isPositive vStr) w prefix padChar
                 else
                     fun (w: int) (v: objnull) ->
-                        GenericNumber.leftJustifyWithNonGFormat (f v) true (GenericNumber.isPositive v) w prefix padChar
+                        let vStr = f v
+                        GenericNumber.leftJustifyWithNonGFormat vStr true (GenericNumber.isPositive vStr) w prefix padChar
 
         /// Right justification handler for f: 'T -> string - basic integer types
         let rightJustify f (prefixForPositives: string) padChar isUnsigned =
             if isUnsigned then
                 if padChar = '0' then
                     fun (w: int) (v: objnull) ->
-                        GenericNumber.rightJustifyWithZeroAsPadChar (f v) true true w prefixForPositives
+                        let vStr = f v
+                        GenericNumber.rightJustifyWithZeroAsPadChar vStr true true w prefixForPositives
                 else
                     Debug.Assert((padChar = ' '))
                     fun (w: int) (v: objnull) ->
                         GenericNumber.rightJustifyWithSpaceAsPadChar (f v) true true w prefixForPositives
             else
                 if padChar = '0' then
                     fun (w: int) (v: objnull) ->
-                        GenericNumber.rightJustifyWithZeroAsPadChar (f v) true (GenericNumber.isPositive v) w prefixForPositives
+                        let vStr = f v
+                        GenericNumber.rightJustifyWithZeroAsPadChar vStr true (GenericNumber.isPositive vStr) w prefixForPositives
 
                 else
                     Debug.Assert((padChar = ' '))
                     fun (w: int) v ->
-                        GenericNumber.rightJustifyWithSpaceAsPadChar (f v) true (GenericNumber.isPositive v) w prefixForPositives
+                        let vStr = f v
+                        GenericNumber.rightJustifyWithSpaceAsPadChar vStr true (GenericNumber.isPositive vStr) w prefixForPositives
 
         /// Computes a new function from 'f' that wraps the basic conversion given
         /// by 'f' with padding for 0, spacing and justification, if the flags specify
@@ -850,7 +853,7 @@ module internal PrintfImpl =
             | _ -> invalidArg (nameof spec) "Invalid integer format"
 
     module FloatAndDecimal = 
-
+        
         let rec toFormattedString fmt (v: obj) = 
             match v with
             | :? single as n -> n.ToString(fmt, CultureInfo.InvariantCulture)
@@ -880,24 +883,29 @@ module internal PrintfImpl =
 
         let noJustification (prefixForPositives: string) = 
             fun (fmt: string) (v: obj) -> 
-                GenericNumber.noJustificationCore (toFormattedString fmt v) (isNumber v) (GenericNumber.isPositive v) prefixForPositives
+                let vStr = toFormattedString fmt v
+                GenericNumber.noJustificationCore vStr (isNumber v) (GenericNumber.isPositive vStr) prefixForPositives
 
         let leftJustify isGFormat (prefix: string) padChar = 
             if isGFormat then
                 fun (fmt: string) (w: int) (v: obj) ->
-                    GenericNumber.leftJustifyWithGFormat (toFormattedString fmt v) (isNumber v) (isInteger v) (GenericNumber.isPositive v) w prefix padChar
+                    let vStr = toFormattedString fmt v
+                    GenericNumber.leftJustifyWithGFormat vStr (isNumber v) (isInteger v) (GenericNumber.isPositive vStr) w prefix padChar
             else
                 fun (fmt: string) (w: int) (v: obj) ->
-                    GenericNumber.leftJustifyWithNonGFormat (toFormattedString fmt v) (isNumber v) (GenericNumber.isPositive v) w prefix padChar  
+                    let vStr = toFormattedString fmt v
+                    GenericNumber.leftJustifyWithNonGFormat vStr (isNumber v) (GenericNumber.isPositive vStr) w prefix padChar
 
         let rightJustify (prefixForPositives: string) padChar =
             if padChar = '0' then
                 fun (fmt: string) (w: int) (v: obj) ->
-                    GenericNumber.rightJustifyWithZeroAsPadChar (toFormattedString fmt v) (isNumber v) (GenericNumber.isPositive v) w prefixForPositives
+                    let vStr = toFormattedString fmt v
+                    GenericNumber.rightJustifyWithZeroAsPadChar vStr (isNumber v) (GenericNumber.isPositive vStr) w prefixForPositives
             else
                 Debug.Assert((padChar = ' '))
                 fun (fmt: string) (w: int) (v: obj) ->
-                    GenericNumber.rightJustifyWithSpaceAsPadChar (toFormattedString fmt v) (isNumber v) (GenericNumber.isPositive v) w prefixForPositives
+                    let vStr = toFormattedString fmt v
+                    GenericNumber.rightJustifyWithSpaceAsPadChar vStr (isNumber v) (GenericNumber.isPositive vStr) w prefixForPositives
 
         let withPadding (spec: FormatSpecifier) getFormat defaultFormat =
             let padChar, prefix = spec.GetPadAndPrefix true 

tests/FSharp.Compiler.ComponentTests/Miscellaneous/FsharpSuiteMigrated.fs

@@ -91,7 +91,7 @@ module TestFrameworkAdapter =
         | LangVersion.Latest  -> "latest", bonusArgs
 
 
-    let singleTestBuildAndRunAuxVersion (folder:string) bonusArgs mode langVersion sessionIsolation = 
+    let singleTestBuildAndRunAuxVersion (folder:string) bonusArgs mode langVersion sessionIsolation =
         let absFolder = Path.Combine(baseFolder,folder)
         let supportedNames, files = 
             match mode with 
@@ -162,9 +162,10 @@ module TestFrameworkAdapter =
 
 
     let singleTestBuildAndRunAux folder bonusArgs mode = singleTestBuildAndRunAuxVersion folder bonusArgs mode LangVersion.Latest
-    let singleTestBuildAndRunVersion folder mode version = singleTestBuildAndRunAuxVersion folder [] mode version
-    let singleTestBuildAndRun folder mode = singleTestBuildAndRunVersion folder mode LangVersion.Latest ScriptSessionIsolation.Shared
-    let singleTestBuildAndRunIsolated folder mode = singleTestBuildAndRunVersion folder mode LangVersion.Latest ScriptSessionIsolation.Isolated
+    let singleTestBuildAndRunVersion folder mode version : unit = singleTestBuildAndRunAuxVersion folder [] mode version ScriptSessionIsolation.Shared
+    let singleTestBuildAndRunVersionIsolated folder mode version : unit = singleTestBuildAndRunAuxVersion folder [] mode version ScriptSessionIsolation.Isolated
+    let singleTestBuildAndRun folder mode : unit = singleTestBuildAndRunVersion folder mode LangVersion.Latest
+    let singleTestBuildAndRunIsolated folder mode : unit = singleTestBuildAndRunVersionIsolated folder mode LangVersion.Latest
 
     let singleVersionedNegTestAux folder bonusArgs version testName =
         singleTestBuildAndRunAuxVersion folder bonusArgs  (NEG_TEST_BUILD testName) version ScriptSessionIsolation.Shared

tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Core/PrintfTests.fs

@@ -75,6 +75,198 @@ 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
+#if NETCOREAPP
+                                "-0.000000"
+#else
+                                "0.000000"
+#endif
+        test "%+f" +0.0         "+0.000000"
+        test "%+f" -0.0
+#if NETCOREAPP
+                                "-0.000000"
+#else
+                                "+0.000000"
+#endif
+
+        test "%f"  +0.0f        "0.000000"
+        test "%f"  -0.0f
+#if NETCOREAPP
+                                "-0.000000"
+#else
+                                "0.000000"
+#endif
+        test "%+f" +0.0f        "+0.000000"
+        test "%+f" -0.0f
+#if NETCOREAPP
+                                "-0.000000"
+#else
+                                "+0.000000"
+#endif
+
+        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 - positive and negative zero - corner cases`` () =
+        test "%-10.3f" +0.0f    "0.000     "
+        test "%-10.3f" -0.0f
+#if NETCOREAPP
+                                "-0.000    "
+#else
+                                "0.000     "
+#endif
+        test "%-+10.3f" +0.0f   "+0.000    "
+        test "%-+10.3f" -0.0f
+#if NETCOREAPP
+                                "-0.000    "
+#else
+                                "+0.000    "
+#endif
+        test "%+020e" +0.0f     "+0000000.000000e+000"
+        test "%+020e" -0.0f
+#if NETCOREAPP
+                                "-0000000.000000e+000"   
+#else
+                                "+0000000.000000e+000"
+#endif
+
+        test "%-10.3f" +0.0    "0.000     "
+        test "%-10.3f" -0.0
+#if NETCOREAPP
+                                "-0.000    "
+#else
+                                "0.000     "
+#endif
+        test "%-+10.3f" +0.0   "+0.000    "
+        test "%-+10.3f" -0.0
+#if NETCOREAPP
+                                "-0.000    "
+#else
+                                "+0.000    "
+#endif
+        test "%+020e" +0.0     "+0000000.000000e+000"
+        test "%+020e" -0.0
+#if NETCOREAPP
+                                "-0000000.000000e+000"   
+#else
+                                "+0000000.000000e+000"
+#endif
+
+
+    [<Fact>]
+    member this.``sign flag - very small positive and negative numbers``() =
+        test "%f"  -0.0000001
+#if NETCOREAPP
+                                "-0.000000"
+#else
+                                "0.000000"
+#endif
+        test "%+f" -0.0000001
+#if NETCOREAPP
+                                "-0.000000"
+#else
+                                "+0.000000"
+#endif
+
+
+        test "%f"  -0.0000001f
+#if NETCOREAPP
+                                "-0.000000"
+#else
+                                "0.000000"
+#endif
+        test "%+f" -0.0000001f
+#if NETCOREAPP
+                                "-0.000000"
+#else
+                                "+0.000000"
+#endif
+
+        test "%f"  -0.0000001M  "0.000000"
+
+        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
+#if NETCOREAPP
+                                    "-00000.000"
+#else
+                                    "000000.000"
+#endif
+
+        test "%010.3f" +0.0f        "000000.000"
+        test "%010.3f" -0.0f
+#if NETCOREAPP
+                                    "-00000.000"
+#else
+                                    "000000.000"
+#endif
+
+        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)

tests/fsharp/core/printf/test.fsx

@@ -17,7 +17,7 @@ let report_failure (s : string) =
 
 // change this to true to run every test case
 // leave as false to randomly execute a subset of cases (this is a very expensive test area)
-let runEveryTest = false
+let runEveryTest = true
 if runEveryTest then stdout.WriteLine "Running every test. Set \"runEveryTest\" to false to reduce runtime"
 else stdout.WriteLine "Running a random subset of tests.  Set \"runEveryTest\" to true to run all tests"