fix(expr): write negative zero as -0, like the legacy engine that runs today
numberToCoercedString keeps the sign of zero; an integer JSON token -0 is still the long 0. The five shared fixtures that say 0 (the newer engine's behaviour) become known differences with the evidence; TestRunnerOracle has no exception left. PORTING.md: track the runner's switch to src/Sdk/Expressions.
- commit
- 9668b06fa1c17e467326e368eec399670379c138
- author
- brice · · 2026-10-12 18:40 UTC
- committer
- brice · · 2026-10-18 14:59 UTC
- parent
- edc4d79
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
9 files · +56 -54
M expr/PORTING.md +21 -24
| @@ -20,15 +20,14 @@ Three kinds of evidence back the tests: | |||
| 20 | 20 | the few assemblies it needs, see `gen/runner/README.md`) and runs it; | |
| 21 | 21 | `TestRunnerOracle` holds the package to it: 34 765 cases with the same | |
| 22 | 22 | result, string form, `toJSON` text and, for errors, the same message (parse | |
| 23 | - | errors included, to the position); the only exceptions are negative zero | |
| 24 | - | (below) and the message texts of Json.NET failures, which must be `fromJSON` | |
| 25 | - | failures on both sides. | |
| 23 | + | errors included, to the position); the only exception is the message text of | |
| 24 | + | Json.NET failures, which must be `fromJSON` failures on both sides. | |
| 26 | 25 | The same driver recorded `testdata/tojson/*.runner`, `ordinal_cases.txt` | |
| 27 | 26 | and `number_cases.txt`, and checked the number formatting on 200 000 random | |
| 28 | 27 | doubles. | |
| 29 | 28 | - `testdata/languageservices/*.json`: the 1 020 cross-language fixtures of | |
| 30 | - | actions/languageservices (see `xlang_test.go`): 1 018 pass, 2 are known | |
| 31 | - | differences with a reason: `case/27` (the runner reports an even `case()` on | |
| 29 | + | actions/languageservices (see `xlang_test.go`): 1 013 pass, 7 are known | |
| 30 | + | differences with a reason: negative zero (5 cases, below), `case/27` (the runner reports an even `case()` on | |
| 32 | 31 | the `(`, not the name) and `syntax-errors/memory-errors/0` (the runner's | |
| 33 | 32 | memory accounting). `testdata/languageservices/SOURCE` has the commit. | |
| 34 | 33 | ||
| @@ -189,11 +188,7 @@ package follows the runner. | |||
| 189 | 188 | `1.23456789012346E+17` (uppercase `E`, at least two exponent digits). | |
| 190 | 189 | So `1e15` is `1E+15`, `123456789012345678` is `1.23456789012346E+17`, | |
| 191 | 190 | `0.000001` is `1E-06`, `5e-324` is `4.94065645841247E-324`, `1e309` is | |
| 192 | - | `Infinity`. Negative zero is `0`: .NET Core prints `-0`, and | |
| 193 | - | `src/Sdk/Expressions/EvaluationResult.cs` (the copy used by the newer | |
| 194 | - | evaluator) maps it back to `0` ("keep our behavior consistent"); the | |
| 195 | - | older `DTExpressions2` copy still prints `-0`, but the shared fixtures | |
| 196 | - | (`coerce_string.json`, `format('{0}', -0)`) say `0`. Checked against the | |
| 191 | + | `Infinity`. Negative zero is `-0` (see the Negative zero item below). Checked against the | |
| 197 | 192 | runner's code on 200 000 random doubles. The number *literal* in an | |
| 198 | 193 | expression is parsed independently of this (see below) and the JSON | |
| 199 | 194 | writer of `toJSON` uses the same conversion (next item). | |
| @@ -255,7 +250,7 @@ package follows the runner. | |||
| 255 | 250 | - [x] **`toJSON`**: resolved, follows the C#. `Sdk/Functions/ToJson.cs` builds | |
| 256 | 251 | the text itself: two-space indentation, `[]`/`{}` for empties, numbers | |
| 257 | 252 | through `ConvertToString` (so `NaN`, `Infinity` and `-Infinity` appear | |
| 258 | - | bare, `1E+21`, 15 significant digits, `-0` is `0`) and strings through | |
| 253 | + | bare, `1E+21`, 15 significant digits, `-0` is `-0`) and strings through | |
| 259 | 254 | `JsonUtility.ToString` (Json.NET defaults: `\b \f \n \r \t`, `\u00XX` | |
| 260 | 255 | for other controls, `\u0085 \u2028 \u2029`, no escaping of `< > & '`, and | |
| 261 | 256 | non-ASCII as is). Checked against the runner's code | |
| @@ -399,19 +394,21 @@ a bug. | |||
| 399 | 394 | worth doing only if the error text matters ("The maximum allowed memory | |
| 400 | 395 | size was exceeded while evaluating the following expression: ...", which | |
| 401 | 396 | needs `ConvertToExpression` of the root, also not ported). | |
| 402 | - | - [ ] **Negative zero.** `-0` is `0` here (`format('{0}', -0)`, `toJSON(-0)`, | |
| 403 | - | `-0` as a result). The legacy engine, the one that runs (see "The two | |
| 404 | - | engines"), prints `-0`: `DTExpressions2/EvaluationResult.ConvertToString` | |
| 405 | - | formats with `G15` and .NET Core writes `-0`. The newer engine | |
| 406 | - | (`Expressions/EvaluationResult.cs`, "keep our behavior consistent") maps | |
| 407 | - | it to `0`, and the cross-language fixture `coerce_string.json` expects | |
| 408 | - | `0`; the choice follows the fixture and the newer engine. Verified by | |
| 409 | - | `engines.mjs`: this is the only result that differs between the engines, | |
| 410 | - | and `TestRunnerOracle` lists the two corpus expressions as a known | |
| 411 | - | difference. If the runner keeps the legacy engine for good and a user | |
| 412 | - | needs `-0`, change `numberToCoercedString`: `x == 0` would return `-0` | |
| 413 | - | for a negative zero (one line, plus the three tests and the two | |
| 414 | - | `.runner` goldens that say `0`). | |
| 397 | + | - [x] **Negative zero**: `-0` is `-0`, as the legacy engine that runs today | |
| 398 | + | (`DTExpressions2/EvaluationResult.ConvertToString` formats with `G15` and | |
| 399 | + | has no special case; .NET Core writes `-0`): `format('{0}', -0)`, `-0` | |
| 400 | + | as a result and `toJSON(-0)` (`toJSON` uses `ConvertToString`). A JSON | |
| 401 | + | integer token `-0` or `-00` is read as the long 0, but `-0.0` keeps its | |
| 402 | + | sign. The newer engine (`Expressions/EvaluationResult.cs`, "keep our | |
| 403 | + | behavior consistent") maps it to `0`, and so do these shared fixtures: | |
| 404 | + | `coerce_string/number#10-11`, `format/format#19-20` and | |
| 405 | + | `tojson/basics#4` (5 cases) are listed in `xlangKnownDifferences` with this | |
| 406 | + | evidence. `engines.mjs` shows it is the only result on which the engines | |
| 407 | + | differ. **Track the runner's switch to `src/Sdk/Expressions` | |
| 408 | + | (`TemplateEvaluatorMismatch` telemetry); re-run | |
| 409 | + | `gen/runner-new/engines.mjs` on each runner release.** If it switches, | |
| 410 | + | change `numberToCoercedString` back to `"0"` (one line) and the tests | |
| 411 | + | that mention `-0`. | |
| 415 | 412 | - [ ] **Comments and constructors in `fromJSON`.** Json.NET accepts `/* */` and | |
| 416 | 413 | `//` comments and `new Date(1)` constructors; the first is read in a | |
| 417 | 414 | surprising way (`/*x*/1` is the string `x`). They fail here. | |
| @@ -20,15 +20,14 @@ Three kinds of evidence back the tests: | |||||
| 20 | the few assemblies it needs, see `gen/runner/README.md`) and runs it; | 20 | the few assemblies it needs, see `gen/runner/README.md`) and runs it; | ||
| 21 | `TestRunnerOracle` holds the package to it: 34 765 cases with the same | 21 | `TestRunnerOracle` holds the package to it: 34 765 cases with the same | ||
| 22 | result, string form, `toJSON` text and, for errors, the same message (parse | 22 | result, string form, `toJSON` text and, for errors, the same message (parse | ||
| 23 | - | errors included, to the position); the only exceptions are negative zero | 23 | + | errors included, to the position); the only exception is the message text of |
| 24 | - | (below) and the message texts of Json.NET failures, which must be `fromJSON` | 24 | + | Json.NET failures, which must be `fromJSON` failures on both sides. |
| 25 | - | failures on both sides. | |||
| 26 | The same driver recorded `testdata/tojson/*.runner`, `ordinal_cases.txt` | 25 | The same driver recorded `testdata/tojson/*.runner`, `ordinal_cases.txt` | ||
| 27 | and `number_cases.txt`, and checked the number formatting on 200 000 random | 26 | and `number_cases.txt`, and checked the number formatting on 200 000 random | ||
| 28 | doubles. | 27 | doubles. | ||
| 29 | - `testdata/languageservices/*.json`: the 1 020 cross-language fixtures of | 28 | - `testdata/languageservices/*.json`: the 1 020 cross-language fixtures of | ||
| 30 | - | actions/languageservices (see `xlang_test.go`): 1 018 pass, 2 are known | 29 | + | actions/languageservices (see `xlang_test.go`): 1 013 pass, 7 are known |
| 31 | - | differences with a reason: `case/27` (the runner reports an even `case()` on | 30 | + | differences with a reason: negative zero (5 cases, below), `case/27` (the runner reports an even `case()` on |
| 32 | the `(`, not the name) and `syntax-errors/memory-errors/0` (the runner's | 31 | the `(`, not the name) and `syntax-errors/memory-errors/0` (the runner's | ||
| 33 | memory accounting). `testdata/languageservices/SOURCE` has the commit. | 32 | memory accounting). `testdata/languageservices/SOURCE` has the commit. | ||
| 34 | 33 | ||||
| @@ -189,11 +188,7 @@ package follows the runner. | |||||
| 189 | `1.23456789012346E+17` (uppercase `E`, at least two exponent digits). | 188 | `1.23456789012346E+17` (uppercase `E`, at least two exponent digits). | ||
| 190 | So `1e15` is `1E+15`, `123456789012345678` is `1.23456789012346E+17`, | 189 | So `1e15` is `1E+15`, `123456789012345678` is `1.23456789012346E+17`, | ||
| 191 | `0.000001` is `1E-06`, `5e-324` is `4.94065645841247E-324`, `1e309` is | 190 | `0.000001` is `1E-06`, `5e-324` is `4.94065645841247E-324`, `1e309` is | ||
| 192 | - | `Infinity`. Negative zero is `0`: .NET Core prints `-0`, and | 191 | + | `Infinity`. Negative zero is `-0` (see the Negative zero item below). Checked against the |
| 193 | - | `src/Sdk/Expressions/EvaluationResult.cs` (the copy used by the newer | |||
| 194 | - | evaluator) maps it back to `0` ("keep our behavior consistent"); the | |||
| 195 | - | older `DTExpressions2` copy still prints `-0`, but the shared fixtures | |||
| 196 | - | (`coerce_string.json`, `format('{0}', -0)`) say `0`. Checked against the | |||
| 197 | runner's code on 200 000 random doubles. The number *literal* in an | 192 | runner's code on 200 000 random doubles. The number *literal* in an | ||
| 198 | expression is parsed independently of this (see below) and the JSON | 193 | expression is parsed independently of this (see below) and the JSON | ||
| 199 | writer of `toJSON` uses the same conversion (next item). | 194 | writer of `toJSON` uses the same conversion (next item). | ||
| @@ -255,7 +250,7 @@ package follows the runner. | |||||
| 255 | - [x] **`toJSON`**: resolved, follows the C#. `Sdk/Functions/ToJson.cs` builds | 250 | - [x] **`toJSON`**: resolved, follows the C#. `Sdk/Functions/ToJson.cs` builds | ||
| 256 | the text itself: two-space indentation, `[]`/`{}` for empties, numbers | 251 | the text itself: two-space indentation, `[]`/`{}` for empties, numbers | ||
| 257 | through `ConvertToString` (so `NaN`, `Infinity` and `-Infinity` appear | 252 | through `ConvertToString` (so `NaN`, `Infinity` and `-Infinity` appear | ||
| 258 | - | bare, `1E+21`, 15 significant digits, `-0` is `0`) and strings through | 253 | + | bare, `1E+21`, 15 significant digits, `-0` is `-0`) and strings through |
| 259 | `JsonUtility.ToString` (Json.NET defaults: `\b \f \n \r \t`, `\u00XX` | 254 | `JsonUtility.ToString` (Json.NET defaults: `\b \f \n \r \t`, `\u00XX` | ||
| 260 | for other controls, `\u0085 \u2028 \u2029`, no escaping of `< > & '`, and | 255 | for other controls, `\u0085 \u2028 \u2029`, no escaping of `< > & '`, and | ||
| 261 | non-ASCII as is). Checked against the runner's code | 256 | non-ASCII as is). Checked against the runner's code | ||
| @@ -399,19 +394,21 @@ a bug. | |||||
| 399 | worth doing only if the error text matters ("The maximum allowed memory | 394 | worth doing only if the error text matters ("The maximum allowed memory | ||
| 400 | size was exceeded while evaluating the following expression: ...", which | 395 | size was exceeded while evaluating the following expression: ...", which | ||
| 401 | needs `ConvertToExpression` of the root, also not ported). | 396 | needs `ConvertToExpression` of the root, also not ported). | ||
| 402 | - | - [ ] **Negative zero.** `-0` is `0` here (`format('{0}', -0)`, `toJSON(-0)`, | 397 | + | - [x] **Negative zero**: `-0` is `-0`, as the legacy engine that runs today |
| 403 | - | `-0` as a result). The legacy engine, the one that runs (see "The two | 398 | + | (`DTExpressions2/EvaluationResult.ConvertToString` formats with `G15` and |
| 404 | - | engines"), prints `-0`: `DTExpressions2/EvaluationResult.ConvertToString` | 399 | + | has no special case; .NET Core writes `-0`): `format('{0}', -0)`, `-0` |
| 405 | - | formats with `G15` and .NET Core writes `-0`. The newer engine | 400 | + | as a result and `toJSON(-0)` (`toJSON` uses `ConvertToString`). A JSON |
| 406 | - | (`Expressions/EvaluationResult.cs`, "keep our behavior consistent") maps | 401 | + | integer token `-0` or `-00` is read as the long 0, but `-0.0` keeps its |
| 407 | - | it to `0`, and the cross-language fixture `coerce_string.json` expects | 402 | + | sign. The newer engine (`Expressions/EvaluationResult.cs`, "keep our |
| 408 | - | `0`; the choice follows the fixture and the newer engine. Verified by | 403 | + | behavior consistent") maps it to `0`, and so do these shared fixtures: |
| 409 | - | `engines.mjs`: this is the only result that differs between the engines, | 404 | + | `coerce_string/number#10-11`, `format/format#19-20` and |
| 410 | - | and `TestRunnerOracle` lists the two corpus expressions as a known | 405 | + | `tojson/basics#4` (5 cases) are listed in `xlangKnownDifferences` with this |
| 411 | - | difference. If the runner keeps the legacy engine for good and a user | 406 | + | evidence. `engines.mjs` shows it is the only result on which the engines |
| 412 | - | needs `-0`, change `numberToCoercedString`: `x == 0` would return `-0` | 407 | + | differ. **Track the runner's switch to `src/Sdk/Expressions` |
| 413 | - | for a negative zero (one line, plus the three tests and the two | 408 | + | (`TemplateEvaluatorMismatch` telemetry); re-run |
| 414 | - | `.runner` goldens that say `0`). | 409 | + | `gen/runner-new/engines.mjs` on each runner release.** If it switches, |
| 410 | + | change `numberToCoercedString` back to `"0"` (one line) and the tests | |||
| 411 | + | that mention `-0`. | |||
| 415 | - [ ] **Comments and constructors in `fromJSON`.** Json.NET accepts `/* */` and | 412 | - [ ] **Comments and constructors in `fromJSON`.** Json.NET accepts `/* */` and | ||
| 416 | `//` comments and `new Date(1)` constructors; the first is read in a | 413 | `//` comments and `new Date(1)` constructors; the first is read in a | ||
| 417 | surprising way (`/*x*/1` is the string `x`). They fail here. | 414 | surprising way (`/*x*/1` is the string `x`). They fail here. | ||
M expr/eval_test.go +2 -2
| @@ -409,7 +409,7 @@ func TestEvalBuiltinFunctions(t *testing.T) { | |||
| 409 | 409 | {"toJSON(1)", "String:1"}, | |
| 410 | 410 | {"toJSON(NaN)", "String:NaN"}, | |
| 411 | 411 | {"toJSON(Infinity)", "String:Infinity"}, | |
| 412 | - | {"toJSON(-0)", "String:0"}, | |
| 412 | + | {"toJSON(-0)", "String:-0"}, | |
| 413 | 413 | {"toJSON('x')", "String:\"x\""}, | |
| 414 | 414 | {"toJSON(1e21)", "String:1E+21"}, | |
| 415 | 415 | {"toJSON(1e-7)", "String:1E-07"}, | |
| @@ -838,7 +838,7 @@ func TestFloatCoercion(t *testing.T) { | |||
| 838 | 838 | want string | |
| 839 | 839 | }{ | |
| 840 | 840 | {0, "0"}, | |
| 841 | - | {math.Copysign(0, -1), "0"}, // see numberToCoercedString | |
| 841 | + | {math.Copysign(0, -1), "-0"}, // as the legacy engine of the runner | |
| 842 | 842 | {1, "1"}, | |
| 843 | 843 | {-1, "-1"}, | |
| 844 | 844 | {1.5, "1.5"}, | |
| @@ -409,7 +409,7 @@ func TestEvalBuiltinFunctions(t *testing.T) { | |||||
| 409 | {"toJSON(1)", "String:1"}, | 409 | {"toJSON(1)", "String:1"}, | ||
| 410 | {"toJSON(NaN)", "String:NaN"}, | 410 | {"toJSON(NaN)", "String:NaN"}, | ||
| 411 | {"toJSON(Infinity)", "String:Infinity"}, | 411 | {"toJSON(Infinity)", "String:Infinity"}, | ||
| 412 | - | {"toJSON(-0)", "String:0"}, | 412 | + | {"toJSON(-0)", "String:-0"}, |
| 413 | {"toJSON('x')", "String:\"x\""}, | 413 | {"toJSON('x')", "String:\"x\""}, | ||
| 414 | {"toJSON(1e21)", "String:1E+21"}, | 414 | {"toJSON(1e21)", "String:1E+21"}, | ||
| 415 | {"toJSON(1e-7)", "String:1E-07"}, | 415 | {"toJSON(1e-7)", "String:1E-07"}, | ||
| @@ -838,7 +838,7 @@ func TestFloatCoercion(t *testing.T) { | |||||
| 838 | want string | 838 | want string | ||
| 839 | }{ | 839 | }{ | ||
| 840 | {0, "0"}, | 840 | {0, "0"}, | ||
| 841 | - | {math.Copysign(0, -1), "0"}, // see numberToCoercedString | 841 | + | {math.Copysign(0, -1), "-0"}, // as the legacy engine of the runner |
| 842 | {1, "1"}, | 842 | {1, "1"}, | ||
| 843 | {-1, "-1"}, | 843 | {-1, "-1"}, | ||
| 844 | {1.5, "1.5"}, | 844 | {1.5, "1.5"}, | ||
M expr/fromjson_runner_test.go +0 -7
| @@ -5,7 +5,6 @@ package expr | |||
| 5 | 5 | import ( | |
| 6 | 6 | "encoding/json" | |
| 7 | 7 | "os" | |
| 8 | - | "regexp" | |
| 9 | 8 | "strings" | |
| 10 | 9 | "testing" | |
| 11 | 10 | ) | |
| @@ -29,9 +28,6 @@ func loadFromJSONCases(tb testing.TB) []fromJSONCase { | |||
| 29 | 28 | return cs | |
| 30 | 29 | } | |
| 31 | 30 | ||
| 32 | - | // negZero matches a written negative zero ("-0" not followed by more number). | |
| 33 | - | var negZero = regexp.MustCompile(`(^|[^0-9.eE+-])-0($|[^0-9.eE])`) | |
| 34 | - | ||
| 35 | 31 | // unportedJSON reports whether the input uses something of Json.NET that is not | |
| 36 | 32 | // ported: comments and constructors. The runner accepts them, this package | |
| 37 | 33 | // fails on them (see PORTING.md). | |
| @@ -53,9 +49,6 @@ func TestFromJSONMatchesRunner(t *testing.T) { | |||
| 53 | 49 | } | |
| 54 | 50 | gotOK := err == nil | |
| 55 | 51 | if gotOK != c.OK || (gotOK && ToJSON(v) != c.Out) { | |
| 56 | - | if gotOK && c.OK && negZero.ReplaceAllString(c.Out, "${1}0${2}") == ToJSON(v) { | |
| 57 | - | continue // negative zero is written 0, see PORTING.md | |
| 58 | - | } | |
| 59 | 52 | if unportedJSON(c.In) { | |
| 60 | 53 | skipped++ | |
| 61 | 54 | continue | |
| @@ -5,7 +5,6 @@ package expr | |||||
| 5 | import ( | 5 | import ( | ||
| 6 | "encoding/json" | 6 | "encoding/json" | ||
| 7 | "os" | 7 | "os" | ||
| 8 | - | "regexp" | |||
| 9 | "strings" | 8 | "strings" | ||
| 10 | "testing" | 9 | "testing" | ||
| 11 | ) | 10 | ) | ||
| @@ -29,9 +28,6 @@ func loadFromJSONCases(tb testing.TB) []fromJSONCase { | |||||
| 29 | return cs | 28 | return cs | ||
| 30 | } | 29 | } | ||
| 31 | 30 | ||||
| 32 | - | // negZero matches a written negative zero ("-0" not followed by more number). | |||
| 33 | - | var negZero = regexp.MustCompile(`(^|[^0-9.eE+-])-0($|[^0-9.eE])`) | |||
| 34 | - | ||||
| 35 | // unportedJSON reports whether the input uses something of Json.NET that is not | 31 | // unportedJSON reports whether the input uses something of Json.NET that is not | ||
| 36 | // ported: comments and constructors. The runner accepts them, this package | 32 | // ported: comments and constructors. The runner accepts them, this package | ||
| 37 | // fails on them (see PORTING.md). | 33 | // fails on them (see PORTING.md). | ||
| @@ -53,9 +49,6 @@ func TestFromJSONMatchesRunner(t *testing.T) { | |||||
| 53 | } | 49 | } | ||
| 54 | gotOK := err == nil | 50 | gotOK := err == nil | ||
| 55 | if gotOK != c.OK || (gotOK && ToJSON(v) != c.Out) { | 51 | if gotOK != c.OK || (gotOK && ToJSON(v) != c.Out) { | ||
| 56 | - | if gotOK && c.OK && negZero.ReplaceAllString(c.Out, "${1}0${2}") == ToJSON(v) { | |||
| 57 | - | continue // negative zero is written 0, see PORTING.md | |||
| 58 | - | } | |||
| 59 | if unportedJSON(c.In) { | 52 | if unportedJSON(c.In) { | ||
| 60 | skipped++ | 53 | skipped++ | ||
| 61 | continue | 54 | continue | ||
M expr/fuzz_test.go +6 -1
| @@ -4,6 +4,7 @@ package expr | |||
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | 6 | "context" | |
| 7 | + | "regexp" | |
| 7 | 8 | "strings" | |
| 8 | 9 | "testing" | |
| 9 | 10 | "unicode/utf8" | |
| @@ -192,7 +193,8 @@ func FuzzFromJSON(f *testing.F) { | |||
| 192 | 193 | if err != nil { | |
| 193 | 194 | t.Fatalf("round trip failed: %v\n%s", err, j) | |
| 194 | 195 | } | |
| 195 | - | if j2 := ToJSON(v2); j2 != j { | |
| 196 | + | // A negative zero is written "-0" and read back as the integer 0. | |
| 197 | + | if j2 := ToJSON(v2); j2 != j && negZeroLine.ReplaceAllString(j, "${1}0${2}") != j2 { | |
| 196 | 198 | t.Fatalf("round trip changed output:\n%s\n%s", j, j2) | |
| 197 | 199 | } | |
| 198 | 200 | }) | |
| @@ -203,3 +205,6 @@ func FuzzFromJSON(f *testing.F) { | |||
| 203 | 205 | func hasNonFinite(j string) bool { | |
| 204 | 206 | return strings.Contains(j, "NaN") || strings.Contains(j, "Infinity") | |
| 205 | 207 | } | |
| 208 | + | ||
| 209 | + | // negZeroLine matches a written negative zero: "-0" alone on its value. | |
| 210 | + | var negZeroLine = regexp.MustCompile(`(^|[\s\[:,])-0($|[\s,\]}])`) | |
| @@ -4,6 +4,7 @@ package expr | |||||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | "context" | 6 | "context" | ||
| 7 | + | "regexp" | |||
| 7 | "strings" | 8 | "strings" | ||
| 8 | "testing" | 9 | "testing" | ||
| 9 | "unicode/utf8" | 10 | "unicode/utf8" | ||
| @@ -192,7 +193,8 @@ func FuzzFromJSON(f *testing.F) { | |||||
| 192 | if err != nil { | 193 | if err != nil { | ||
| 193 | t.Fatalf("round trip failed: %v\n%s", err, j) | 194 | t.Fatalf("round trip failed: %v\n%s", err, j) | ||
| 194 | } | 195 | } | ||
| 195 | - | if j2 := ToJSON(v2); j2 != j { | 196 | + | // A negative zero is written "-0" and read back as the integer 0. |
| 197 | + | if j2 := ToJSON(v2); j2 != j && negZeroLine.ReplaceAllString(j, "${1}0${2}") != j2 { | |||
| 196 | t.Fatalf("round trip changed output:\n%s\n%s", j, j2) | 198 | t.Fatalf("round trip changed output:\n%s\n%s", j, j2) | ||
| 197 | } | 199 | } | ||
| 198 | }) | 200 | }) | ||
| @@ -203,3 +205,6 @@ func FuzzFromJSON(f *testing.F) { | |||||
| 203 | func hasNonFinite(j string) bool { | 205 | func hasNonFinite(j string) bool { | ||
| 204 | return strings.Contains(j, "NaN") || strings.Contains(j, "Infinity") | 206 | return strings.Contains(j, "NaN") || strings.Contains(j, "Infinity") | ||
| 205 | } | 207 | } | ||
| 208 | + | ||||
| 209 | + | // negZeroLine matches a written negative zero: "-0" alone on its value. | |||
| 210 | + | var negZeroLine = regexp.MustCompile(`(^|[\s\[:,])-0($|[\s,\]}])`) | |||
M expr/json.go +3 -0
| @@ -210,6 +210,9 @@ func (p *jsonParser) number() (Value, error) { | |||
| 210 | 210 | } | |
| 211 | 211 | } | |
| 212 | 212 | f, _ := strconv.ParseFloat(tok, 64) // range errors give ±Inf | |
| 213 | + | if f == 0 && isBigInteger(body) { | |
| 214 | + | f = 0 // an integer token is read as a long: "-0" and "-00" are plain 0 | |
| 215 | + | } | |
| 213 | 216 | return Number(f), nil | |
| 214 | 217 | } | |
| 215 | 218 | ||
| @@ -210,6 +210,9 @@ func (p *jsonParser) number() (Value, error) { | |||||
| 210 | } | 210 | } | ||
| 211 | } | 211 | } | ||
| 212 | f, _ := strconv.ParseFloat(tok, 64) // range errors give ±Inf | 212 | f, _ := strconv.ParseFloat(tok, 64) // range errors give ±Inf | ||
| 213 | + | if f == 0 && isBigInteger(body) { | |||
| 214 | + | f = 0 // an integer token is read as a long: "-0" and "-00" are plain 0 | |||
| 215 | + | } | |||
| 213 | return Number(f), nil | 216 | return Number(f), nil | ||
| 214 | } | 217 | } | ||
| 215 | 218 | ||||
M expr/json_test.go +10 -6
| @@ -154,12 +154,16 @@ func TestFromJSONNumbers(t *testing.T) { | |||
| 154 | 154 | t.Errorf("FromJSON(%q) = %v, want %v", in, got, want) | |
| 155 | 155 | } | |
| 156 | 156 | } | |
| 157 | - | v, _ := FromJSON([]byte("-0")) | |
| 157 | + | // Json.NET reads the integer "-0" as 0, but "-0.0" is a float with a sign. | |
| 158 | + | if v, _ := FromJSON([]byte("-0")); math.Signbit(float64(v.(Number))) { | |
| 159 | + | t.Error(`"-0" kept its sign`) | |
| 160 | + | } | |
| 161 | + | v, _ := FromJSON([]byte("-0.0")) | |
| 158 | 162 | if !math.Signbit(float64(v.(Number))) { | |
| 159 | - | t.Error("-0 lost its sign") | |
| 163 | + | t.Error("-0.0 lost its sign") | |
| 160 | 164 | } | |
| 161 | - | if got := ToJSON(v); got != "0" { | |
| 162 | - | t.Errorf("ToJSON(-0) = %q", got) | |
| 165 | + | if got := ToJSON(v); got != "-0" { | |
| 166 | + | t.Errorf("ToJSON(-0.0) = %q", got) | |
| 163 | 167 | } | |
| 164 | 168 | } | |
| 165 | 169 | ||
| @@ -250,7 +254,7 @@ func TestToJSONFormatting(t *testing.T) { | |||
| 250 | 254 | {Number(math.NaN()), "NaN"}, | |
| 251 | 255 | {Number(math.Inf(1)), "Infinity"}, | |
| 252 | 256 | {Number(math.Inf(-1)), "-Infinity"}, | |
| 253 | - | {Number(math.Copysign(0, -1)), "0"}, | |
| 257 | + | {Number(math.Copysign(0, -1)), "-0"}, | |
| 254 | 258 | {Number(1e21), "1E+21"}, | |
| 255 | 259 | {Number(1e-7), "1E-07"}, | |
| 256 | 260 | {Number(123456789), "123456789"}, | |
| @@ -343,7 +347,7 @@ func TestCoerceAndTruthy(t *testing.T) { | |||
| 343 | 347 | {Boolean(true), "true", true}, | |
| 344 | 348 | {Boolean(false), "false", false}, | |
| 345 | 349 | {Number(0), "0", false}, | |
| 346 | - | {Number(math.Copysign(0, -1)), "0", false}, | |
| 350 | + | {Number(math.Copysign(0, -1)), "-0", false}, | |
| 347 | 351 | {Number(math.NaN()), "NaN", false}, | |
| 348 | 352 | {Number(2), "2", true}, | |
| 349 | 353 | {Number(math.Inf(-1)), "-Infinity", true}, | |
| @@ -154,12 +154,16 @@ func TestFromJSONNumbers(t *testing.T) { | |||||
| 154 | t.Errorf("FromJSON(%q) = %v, want %v", in, got, want) | 154 | t.Errorf("FromJSON(%q) = %v, want %v", in, got, want) | ||
| 155 | } | 155 | } | ||
| 156 | } | 156 | } | ||
| 157 | - | v, _ := FromJSON([]byte("-0")) | 157 | + | // Json.NET reads the integer "-0" as 0, but "-0.0" is a float with a sign. |
| 158 | + | if v, _ := FromJSON([]byte("-0")); math.Signbit(float64(v.(Number))) { | |||
| 159 | + | t.Error(`"-0" kept its sign`) | |||
| 160 | + | } | |||
| 161 | + | v, _ := FromJSON([]byte("-0.0")) | |||
| 158 | if !math.Signbit(float64(v.(Number))) { | 162 | if !math.Signbit(float64(v.(Number))) { | ||
| 159 | - | t.Error("-0 lost its sign") | 163 | + | t.Error("-0.0 lost its sign") |
| 160 | } | 164 | } | ||
| 161 | - | if got := ToJSON(v); got != "0" { | 165 | + | if got := ToJSON(v); got != "-0" { |
| 162 | - | t.Errorf("ToJSON(-0) = %q", got) | 166 | + | t.Errorf("ToJSON(-0.0) = %q", got) |
| 163 | } | 167 | } | ||
| 164 | } | 168 | } | ||
| 165 | 169 | ||||
| @@ -250,7 +254,7 @@ func TestToJSONFormatting(t *testing.T) { | |||||
| 250 | {Number(math.NaN()), "NaN"}, | 254 | {Number(math.NaN()), "NaN"}, | ||
| 251 | {Number(math.Inf(1)), "Infinity"}, | 255 | {Number(math.Inf(1)), "Infinity"}, | ||
| 252 | {Number(math.Inf(-1)), "-Infinity"}, | 256 | {Number(math.Inf(-1)), "-Infinity"}, | ||
| 253 | - | {Number(math.Copysign(0, -1)), "0"}, | 257 | + | {Number(math.Copysign(0, -1)), "-0"}, |
| 254 | {Number(1e21), "1E+21"}, | 258 | {Number(1e21), "1E+21"}, | ||
| 255 | {Number(1e-7), "1E-07"}, | 259 | {Number(1e-7), "1E-07"}, | ||
| 256 | {Number(123456789), "123456789"}, | 260 | {Number(123456789), "123456789"}, | ||
| @@ -343,7 +347,7 @@ func TestCoerceAndTruthy(t *testing.T) { | |||||
| 343 | {Boolean(true), "true", true}, | 347 | {Boolean(true), "true", true}, | ||
| 344 | {Boolean(false), "false", false}, | 348 | {Boolean(false), "false", false}, | ||
| 345 | {Number(0), "0", false}, | 349 | {Number(0), "0", false}, | ||
| 346 | - | {Number(math.Copysign(0, -1)), "0", false}, | 350 | + | {Number(math.Copysign(0, -1)), "-0", false}, |
| 347 | {Number(math.NaN()), "NaN", false}, | 351 | {Number(math.NaN()), "NaN", false}, | ||
| 348 | {Number(2), "2", true}, | 352 | {Number(2), "2", true}, | ||
| 349 | {Number(math.Inf(-1)), "-Infinity", true}, | 353 | {Number(math.Inf(-1)), "-Infinity", true}, | ||
M expr/number.go +7 -4
| @@ -26,10 +26,10 @@ var ( | |||
| 26 | 26 | // value satisfies -5 < X < 15, and as d.dddE+XX otherwise, with the trailing | |
| 27 | 27 | // zeros of the mantissa removed and at least two exponent digits. NaN is | |
| 28 | 28 | // "NaN" and the infinities are "Infinity" and "-Infinity". Negative zero is | |
| 29 | - | // "0": .NET Core prints "-0", which Sdk/Expressions/EvaluationResult.cs | |
| 30 | - | // (ConvertToString, "keep our behavior consistent") and the shared fixtures | |
| 31 | - | // (coerce_string.json, format('{0}', -0)) undo; the older | |
| 32 | - | // DTExpressions2 copy of the runner lacks that fix and prints "-0". | |
| 29 | + | // "-0", as .NET Core prints it and as the legacy engine of the runner | |
| 30 | + | // (DTExpressions2/EvaluationResult.ConvertToString, the one that runs today) | |
| 31 | + | // leaves it; the newer engine (Expressions/EvaluationResult.cs) maps it to "0". | |
| 32 | + | // See PORTING.md. | |
| 33 | 33 | func numberToCoercedString(x float64) string { | |
| 34 | 34 | switch { | |
| 35 | 35 | case math.IsNaN(x): | |
| @@ -39,6 +39,9 @@ func numberToCoercedString(x float64) string { | |||
| 39 | 39 | case math.IsInf(x, -1): | |
| 40 | 40 | return "-Infinity" | |
| 41 | 41 | case x == 0: | |
| 42 | + | if math.Signbit(x) { | |
| 43 | + | return "-0" | |
| 44 | + | } | |
| 42 | 45 | return "0" | |
| 43 | 46 | } | |
| 44 | 47 | sign := "" | |
| @@ -26,10 +26,10 @@ var ( | |||||
| 26 | // value satisfies -5 < X < 15, and as d.dddE+XX otherwise, with the trailing | 26 | // value satisfies -5 < X < 15, and as d.dddE+XX otherwise, with the trailing | ||
| 27 | // zeros of the mantissa removed and at least two exponent digits. NaN is | 27 | // zeros of the mantissa removed and at least two exponent digits. NaN is | ||
| 28 | // "NaN" and the infinities are "Infinity" and "-Infinity". Negative zero is | 28 | // "NaN" and the infinities are "Infinity" and "-Infinity". Negative zero is | ||
| 29 | - | // "0": .NET Core prints "-0", which Sdk/Expressions/EvaluationResult.cs | 29 | + | // "-0", as .NET Core prints it and as the legacy engine of the runner |
| 30 | - | // (ConvertToString, "keep our behavior consistent") and the shared fixtures | 30 | + | // (DTExpressions2/EvaluationResult.ConvertToString, the one that runs today) |
| 31 | - | // (coerce_string.json, format('{0}', -0)) undo; the older | 31 | + | // leaves it; the newer engine (Expressions/EvaluationResult.cs) maps it to "0". |
| 32 | - | // DTExpressions2 copy of the runner lacks that fix and prints "-0". | 32 | + | // See PORTING.md. |
| 33 | func numberToCoercedString(x float64) string { | 33 | func numberToCoercedString(x float64) string { | ||
| 34 | switch { | 34 | switch { | ||
| 35 | case math.IsNaN(x): | 35 | case math.IsNaN(x): | ||
| @@ -39,6 +39,9 @@ func numberToCoercedString(x float64) string { | |||||
| 39 | case math.IsInf(x, -1): | 39 | case math.IsInf(x, -1): | ||
| 40 | return "-Infinity" | 40 | return "-Infinity" | ||
| 41 | case x == 0: | 41 | case x == 0: | ||
| 42 | + | if math.Signbit(x) { | |||
| 43 | + | return "-0" | |||
| 44 | + | } | |||
| 42 | return "0" | 45 | return "0" | ||
| 43 | } | 46 | } | ||
| 44 | sign := "" | 47 | sign := "" | ||
M expr/runner_strict_test.go +2 -10
| @@ -10,16 +10,8 @@ import ( | |||
| 10 | 10 | ||
| 11 | 11 | // knownRunnerDifference classifies the answers of the corpus on which this | |
| 12 | 12 | // package knowingly differs from the runner. It returns the reason, or "". | |
| 13 | - | func knownRunnerDifference(g, r runnerCase) string { | |
| 14 | - | switch r.E { | |
| 15 | - | case "-0", "toJSON(-0)": | |
| 16 | - | // .NET Core formats negative zero as "-0". The runner's newer copy | |
| 17 | - | // (src/Sdk/Expressions/EvaluationResult.cs) maps it back to "0" and the | |
| 18 | - | // shared fixtures (coerce_string.json) say "0", so "0" is kept. | |
| 19 | - | return "negative zero is written 0" | |
| 20 | - | } | |
| 21 | - | return "" | |
| 22 | - | } | |
| 13 | + | // There is none left: negative zero is written "-0" like the legacy engine. | |
| 14 | + | func knownRunnerDifference(g, r runnerCase) string { return "" } | |
| 23 | 15 | ||
| 24 | 16 | // TestRunnerOracle holds the package to the answers of the runner's own code on | |
| 25 | 17 | // the 34 765 expressions of the oracle corpus: same result kind, string form | |
| @@ -10,16 +10,8 @@ import ( | |||||
| 10 | 10 | ||||
| 11 | // knownRunnerDifference classifies the answers of the corpus on which this | 11 | // knownRunnerDifference classifies the answers of the corpus on which this | ||
| 12 | // package knowingly differs from the runner. It returns the reason, or "". | 12 | // package knowingly differs from the runner. It returns the reason, or "". | ||
| 13 | - | func knownRunnerDifference(g, r runnerCase) string { | 13 | + | // There is none left: negative zero is written "-0" like the legacy engine. |
| 14 | - | switch r.E { | 14 | + | func knownRunnerDifference(g, r runnerCase) string { return "" } |
| 15 | - | case "-0", "toJSON(-0)": | |||
| 16 | - | // .NET Core formats negative zero as "-0". The runner's newer copy | |||
| 17 | - | // (src/Sdk/Expressions/EvaluationResult.cs) maps it back to "0" and the | |||
| 18 | - | // shared fixtures (coerce_string.json) say "0", so "0" is kept. | |||
| 19 | - | return "negative zero is written 0" | |||
| 20 | - | } | |||
| 21 | - | return "" | |||
| 22 | - | } | |||
| 23 | 15 | ||||
| 24 | // TestRunnerOracle holds the package to the answers of the runner's own code on | 16 | // TestRunnerOracle holds the package to the answers of the runner's own code on | ||
| 25 | // the 34 765 expressions of the oracle corpus: same result kind, string form | 17 | // the 34 765 expressions of the oracle corpus: same result kind, string form | ||
M expr/xlang_test.go +5 -0
| @@ -59,6 +59,11 @@ type xlangCase struct { | |||
| 59 | 59 | // xlangKnownDifferences maps "file: group (expr prefix)" keys to the reason | |
| 60 | 60 | // the fixture is not satisfied. Keep it empty if possible. | |
| 61 | 61 | var xlangKnownDifferences = map[string]string{ | |
| 62 | + | "coerce_string: number#10": "negative zero: the fixture says \"0\" (as the newer engine, src/Sdk/Expressions/EvaluationResult.cs); the legacy engine that runs today prints \"-0\" (DTExpressions2/EvaluationResult.cs, no -0 special case), checked with gen/runner and engines.mjs", | |
| 63 | + | "format: format#19": "negative zero (format('{0}', -0)), same reason as coerce_string number#10", | |
| 64 | + | "format: format#20": "negative zero (-0.0 literal), same reason", | |
| 65 | + | "tojson: basics#4": "negative zero (tojson(-0)), same reason: toJson writes numbers with ConvertToString", | |
| 66 | + | "coerce_string: number#11": "negative zero (-0.0 literal), same reason as number#10", | |
| 62 | 67 | "case: case#27": "the runner reports an even number of case() arguments on the '(' token (position 5), not on the name: ExpressionParser.FlushTopEndParameters passes the popped StartParameters token; checked by running the runner's code", | |
| 63 | 68 | "syntax-errors: memory-errors#0": "memory accounting differs from the runner's EvaluationMemory", | |
| 64 | 69 | } | |
| @@ -59,6 +59,11 @@ type xlangCase struct { | |||||
| 59 | // xlangKnownDifferences maps "file: group (expr prefix)" keys to the reason | 59 | // xlangKnownDifferences maps "file: group (expr prefix)" keys to the reason | ||
| 60 | // the fixture is not satisfied. Keep it empty if possible. | 60 | // the fixture is not satisfied. Keep it empty if possible. | ||
| 61 | var xlangKnownDifferences = map[string]string{ | 61 | var xlangKnownDifferences = map[string]string{ | ||
| 62 | + | "coerce_string: number#10": "negative zero: the fixture says \"0\" (as the newer engine, src/Sdk/Expressions/EvaluationResult.cs); the legacy engine that runs today prints \"-0\" (DTExpressions2/EvaluationResult.cs, no -0 special case), checked with gen/runner and engines.mjs", | |||
| 63 | + | "format: format#19": "negative zero (format('{0}', -0)), same reason as coerce_string number#10", | |||
| 64 | + | "format: format#20": "negative zero (-0.0 literal), same reason", | |||
| 65 | + | "tojson: basics#4": "negative zero (tojson(-0)), same reason: toJson writes numbers with ConvertToString", | |||
| 66 | + | "coerce_string: number#11": "negative zero (-0.0 literal), same reason as number#10", | |||
| 62 | "case: case#27": "the runner reports an even number of case() arguments on the '(' token (position 5), not on the name: ExpressionParser.FlushTopEndParameters passes the popped StartParameters token; checked by running the runner's code", | 67 | "case: case#27": "the runner reports an even number of case() arguments on the '(' token (position 5), not on the name: ExpressionParser.FlushTopEndParameters passes the popped StartParameters token; checked by running the runner's code", | ||
| 63 | "syntax-errors: memory-errors#0": "memory accounting differs from the runner's EvaluationMemory", | 68 | "syntax-errors: memory-errors#0": "memory accounting differs from the runner's EvaluationMemory", | ||
| 64 | } | 69 | } | ||