diff --git a/docs/release-notes/.FSharp.Core/11.0.100.md b/docs/release-notes/.FSharp.Core/11.0.100.md index d17942708c9..fcc5c0a07d9 100644 --- a/docs/release-notes/.FSharp.Core/11.0.100.md +++ b/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 diff --git a/src/FSharp.Core/printf.fs b/src/FSharp.Core/printf.fs index 8dfaf52b680..3d503139815 100644 --- a/src/FSharp.Core/printf.fs +++ b/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,17 +788,20 @@ 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) -> @@ -808,12 +809,14 @@ module internal PrintfImpl = 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 diff --git a/tests/FSharp.Compiler.ComponentTests/Miscellaneous/FsharpSuiteMigrated.fs b/tests/FSharp.Compiler.ComponentTests/Miscellaneous/FsharpSuiteMigrated.fs index b976f60d438..7fb9a614885 100644 --- a/tests/FSharp.Compiler.ComponentTests/Miscellaneous/FsharpSuiteMigrated.fs +++ b/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 diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Core/PrintfTests.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Core/PrintfTests.fs index 187fa62c60e..ab3c17d9d20 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Core/PrintfTests.fs +++ b/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 + [] + 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" + + [] + 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" + + [] + 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 + + + [] + 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" + + [] + 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" + + [] + 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) + [] + 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" + + [] + 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" + [] member _.``union case formatting`` () = Assert.AreEqual("CaseOne", sprintf "%A" CaseOne) diff --git a/tests/fsharp/core/printf/test.fsx b/tests/fsharp/core/printf/test.fsx index 3ebe2c9ba33..973e26a28ec 100644 --- a/tests/fsharp/core/printf/test.fsx +++ b/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"