From d33668fa91b0f64cb6f69f315a2b2e8609fdbee0 Mon Sep 17 00:00:00 2001 From: Feoramund <161657516+Feoramund@users.noreply.github.com> Date: Tue, 4 Jun 2024 13:24:46 -0400 Subject: [PATCH 1/5] Fix partial parsing of "infinity" in `parse_f64_prefix` It was previously reporting an invalid number of characters parsed for any string other than "inf", "+inf", or "-inf". --- core/strconv/strconv.odin | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/core/strconv/strconv.odin b/core/strconv/strconv.odin index 94842617e..990b2be2f 100644 --- a/core/strconv/strconv.odin +++ b/core/strconv/strconv.odin @@ -878,13 +878,10 @@ parse_f64_prefix :: proc(str: string) -> (value: f64, nr: int, ok: bool) { s = s[1:] fallthrough case 'i', 'I': - n = common_prefix_len_ignore_case(s, "infinity") - if 3 < n && n < 8 { // "inf" or "infinity" - n = 3 - } - if n == 3 || n == 8 { + m := common_prefix_len_ignore_case(s, "infinity") + if m == 3 || m == 8 { // "inf" or "infinity" f = 0h7ff00000_00000000 if sign == 1 else 0hfff00000_00000000 - n = nsign + 3 + n = nsign + m ok = true return } From 7d670f65624c7c1f0d9a93808a99e259b95bd4b6 Mon Sep 17 00:00:00 2001 From: Feoramund <161657516+Feoramund@users.noreply.github.com> Date: Tue, 4 Jun 2024 13:39:31 -0400 Subject: [PATCH 2/5] Add initial test suite for `core:strconv` --- tests/core/Makefile | 4 ++ tests/core/build.bat | 5 ++ tests/core/strconv/test_core_strconv.odin | 79 +++++++++++++++++++++++ 3 files changed, 88 insertions(+) create mode 100644 tests/core/strconv/test_core_strconv.odin diff --git a/tests/core/Makefile b/tests/core/Makefile index 85f3783b4..79af9c3c7 100644 --- a/tests/core/Makefile +++ b/tests/core/Makefile @@ -25,6 +25,7 @@ all_bsd: download_test_assets \ reflect_test \ runtime_test \ slice_test \ + strconv_test \ strings_test \ thread_test \ time_test @@ -98,6 +99,9 @@ runtime_test: slice_test: $(ODIN) test slice $(COMMON) -out:test_core_slice +strconv_test: + $(ODIN) test strconv $(COMMON) -out:test_core_strconv + strings_test: $(ODIN) test strings $(COMMON) -out:test_core_strings diff --git a/tests/core/build.bat b/tests/core/build.bat index 67ac10f86..18506408a 100644 --- a/tests/core/build.bat +++ b/tests/core/build.bat @@ -103,6 +103,11 @@ echo Running core:slice tests echo --- %PATH_TO_ODIN% test slice %COMMON% -out:test_core_slice.exe || exit /b +echo --- +echo Running core:strconv tests +echo --- +%PATH_TO_ODIN% test strconv %COMMON% -out:test_core_strconv.exe || exit /b + echo --- echo Running core:strings tests echo --- diff --git a/tests/core/strconv/test_core_strconv.odin b/tests/core/strconv/test_core_strconv.odin new file mode 100644 index 000000000..9d713a356 --- /dev/null +++ b/tests/core/strconv/test_core_strconv.odin @@ -0,0 +1,79 @@ +package test_core_strconv + +import "core:math" +import "core:strconv" +import "core:testing" + +@(test) +test_infinity :: proc(t: ^testing.T) { + pos_inf := math.inf_f64(+1) + neg_inf := math.inf_f64(-1) + + n: int + s := "infinity" + + for i in 1 ..< len(s) + 1 { + ss := s[:i] + f, ok := strconv.parse_f64(ss, &n) + if i == 3 { // "inf" + testing.expect_value(t, f, pos_inf) + testing.expect_value(t, n, 3) + testing.expect_value(t, ok, true) + } else if i == 8 { // "infinity" + testing.expect_value(t, f, pos_inf) + testing.expect_value(t, n, 8) + testing.expect_value(t, ok, true) + } else { // invalid substring + testing.expect_value(t, f, 0) + testing.expect_value(t, n, 0) + testing.expect_value(t, ok, false) + } + } + + s = "+infinity" + for i in 1 ..< len(s) + 1 { + ss := s[:i] + f, ok := strconv.parse_f64(ss, &n) + if i == 4 { // "+inf" + testing.expect_value(t, f, pos_inf) + testing.expect_value(t, n, 4) + testing.expect_value(t, ok, true) + } else if i == 9 { // "+infinity" + testing.expect_value(t, f, pos_inf) + testing.expect_value(t, n, 9) + testing.expect_value(t, ok, true) + } else { // invalid substring + testing.expect_value(t, f, 0) + testing.expect_value(t, n, 0) + testing.expect_value(t, ok, false) + } + } + + s = "-infinity" + for i in 1 ..< len(s) + 1 { + ss := s[:i] + f, ok := strconv.parse_f64(ss, &n) + if i == 4 { // "-inf" + testing.expect_value(t, f, neg_inf) + testing.expect_value(t, n, 4) + testing.expect_value(t, ok, true) + } else if i == 9 { // "-infinity" + testing.expect_value(t, f, neg_inf) + testing.expect_value(t, n, 9) + testing.expect_value(t, ok, true) + } else { // invalid substring + testing.expect_value(t, f, 0) + testing.expect_value(t, n, 0) + testing.expect_value(t, ok, false) + } + } + + // Make sure odd casing works. + batch := [?]string {"INFiniTY", "iNfInItY", "InFiNiTy"} + for ss in batch { + f, ok := strconv.parse_f64(ss, &n) + testing.expect_value(t, f, pos_inf) + testing.expect_value(t, n, 8) + testing.expect_value(t, ok, true) + } +} From c656a9e4cdd566b1aa473227d9d9a35b3c6812d3 Mon Sep 17 00:00:00 2001 From: Feoramund <161657516+Feoramund@users.noreply.github.com> Date: Tue, 4 Jun 2024 14:08:19 -0400 Subject: [PATCH 3/5] Fix and subsume `test_issue_2087` into `strconv` test suite The full "infinity" strings were expected to be partial consumes, but this is not the case. That has been fixed and the relevant extra tests from that file have been added to this test suite. Fixes #2670 --- tests/core/strconv/test_core_strconv.odin | 60 ++++++++++++++++++++++ tests/issues/test_issue_2087.odin | 62 ----------------------- 2 files changed, 60 insertions(+), 62 deletions(-) delete mode 100644 tests/issues/test_issue_2087.odin diff --git a/tests/core/strconv/test_core_strconv.odin b/tests/core/strconv/test_core_strconv.odin index 9d713a356..d33ac06b6 100644 --- a/tests/core/strconv/test_core_strconv.odin +++ b/tests/core/strconv/test_core_strconv.odin @@ -4,6 +4,56 @@ import "core:math" import "core:strconv" import "core:testing" +@(test) +test_float :: proc(t: ^testing.T) { + n: int + f: f64 + ok: bool + + f, ok = strconv.parse_f64("1.2", &n) + testing.expect_value(t, f, 1.2) + testing.expect_value(t, n, 3) + testing.expect_value(t, ok, true) + + f, ok = strconv.parse_f64("1.2a", &n) + testing.expect_value(t, f, 1.2) + testing.expect_value(t, n, 3) + testing.expect_value(t, ok, false) + + f, ok = strconv.parse_f64("+", &n) + testing.expect_value(t, f, 0) + testing.expect_value(t, n, 0) + testing.expect_value(t, ok, false) + + f, ok = strconv.parse_f64("-", &n) + testing.expect_value(t, f, 0) + testing.expect_value(t, n, 0) + testing.expect_value(t, ok, false) + +} + +@(test) +test_nan :: proc(t: ^testing.T) { + n: int + f: f64 + ok: bool + + f, ok = strconv.parse_f64("nan", &n) + testing.expect_value(t, math.classify(f), math.Float_Class.NaN) + testing.expect_value(t, n, 3) + testing.expect_value(t, ok, true) + + f, ok = strconv.parse_f64("nAN", &n) + testing.expect_value(t, math.classify(f), math.Float_Class.NaN) + testing.expect_value(t, n, 3) + testing.expect_value(t, ok, true) + + f, ok = strconv.parse_f64("Nani", &n) + testing.expect_value(t, math.classify(f), math.Float_Class.NaN) + testing.expect_value(t, n, 3) + testing.expect_value(t, ok, false) +} + @(test) test_infinity :: proc(t: ^testing.T) { pos_inf := math.inf_f64(+1) @@ -19,14 +69,17 @@ test_infinity :: proc(t: ^testing.T) { testing.expect_value(t, f, pos_inf) testing.expect_value(t, n, 3) testing.expect_value(t, ok, true) + testing.expect_value(t, math.classify(f), math.Float_Class.Inf) } else if i == 8 { // "infinity" testing.expect_value(t, f, pos_inf) testing.expect_value(t, n, 8) testing.expect_value(t, ok, true) + testing.expect_value(t, math.classify(f), math.Float_Class.Inf) } else { // invalid substring testing.expect_value(t, f, 0) testing.expect_value(t, n, 0) testing.expect_value(t, ok, false) + testing.expect_value(t, math.classify(f), math.Float_Class.Zero) } } @@ -38,14 +91,17 @@ test_infinity :: proc(t: ^testing.T) { testing.expect_value(t, f, pos_inf) testing.expect_value(t, n, 4) testing.expect_value(t, ok, true) + testing.expect_value(t, math.classify(f), math.Float_Class.Inf) } else if i == 9 { // "+infinity" testing.expect_value(t, f, pos_inf) testing.expect_value(t, n, 9) testing.expect_value(t, ok, true) + testing.expect_value(t, math.classify(f), math.Float_Class.Inf) } else { // invalid substring testing.expect_value(t, f, 0) testing.expect_value(t, n, 0) testing.expect_value(t, ok, false) + testing.expect_value(t, math.classify(f), math.Float_Class.Zero) } } @@ -57,14 +113,17 @@ test_infinity :: proc(t: ^testing.T) { testing.expect_value(t, f, neg_inf) testing.expect_value(t, n, 4) testing.expect_value(t, ok, true) + testing.expect_value(t, math.classify(f), math.Float_Class.Neg_Inf) } else if i == 9 { // "-infinity" testing.expect_value(t, f, neg_inf) testing.expect_value(t, n, 9) testing.expect_value(t, ok, true) + testing.expect_value(t, math.classify(f), math.Float_Class.Neg_Inf) } else { // invalid substring testing.expect_value(t, f, 0) testing.expect_value(t, n, 0) testing.expect_value(t, ok, false) + testing.expect_value(t, math.classify(f), math.Float_Class.Zero) } } @@ -75,5 +134,6 @@ test_infinity :: proc(t: ^testing.T) { testing.expect_value(t, f, pos_inf) testing.expect_value(t, n, 8) testing.expect_value(t, ok, true) + testing.expect_value(t, math.classify(f), math.Float_Class.Inf) } } diff --git a/tests/issues/test_issue_2087.odin b/tests/issues/test_issue_2087.odin deleted file mode 100644 index 26b6d487d..000000000 --- a/tests/issues/test_issue_2087.odin +++ /dev/null @@ -1,62 +0,0 @@ -// Tests issue #2087 https://github.com/odin-lang/Odin/issues/2087 -package test_issues - -import "core:math" -import "core:strconv" -import "core:testing" - -@(test) -test_parse_float :: proc(t: ^testing.T) { - { - f, ok := strconv.parse_f64("1.2") - testing.expect(t, ok && f == 1.2, "expected f64(1.2), fully consumed") - f, ok = strconv.parse_f64("1.2a") - testing.expect(t, !ok && f == 1.2, "expected f64(1.2), partially consumed") - f, ok = strconv.parse_f64("+") - testing.expect(t, !ok && f == 0.0, "expected f64(0.0), with ok=false") - f, ok = strconv.parse_f64("-") - testing.expect(t, !ok && f == 0.0, "expected f64(0.0), with ok=false") - - - f, ok = strconv.parse_f64("inf") - testing.expect(t, ok && math.classify(f) == math.Float_Class.Inf, "expected f64(+inf), fully consumed") - f, ok = strconv.parse_f64("+inf") - testing.expect(t, ok && math.classify(f) == math.Float_Class.Inf, "expected f64(+inf), fully consumed") - f, ok = strconv.parse_f64("-inf") - testing.expect(t, ok && math.classify(f) == math.Float_Class.Neg_Inf, "expected f64(-inf), fully consumed") - f, ok = strconv.parse_f64("inFinity") - testing.expect(t, !ok && math.classify(f) == math.Float_Class.Inf, "expected f64(+inf), partially consumed") - f, ok = strconv.parse_f64("+InFinity") - testing.expect(t, !ok && math.classify(f) == math.Float_Class.Inf, "expected f64(+inf), partially consumed") - f, ok = strconv.parse_f64("-InfiniTy") - testing.expect(t, !ok && math.classify(f) == math.Float_Class.Neg_Inf, "expected f64(-inf), partially consumed") - f, ok = strconv.parse_f64("nan") - testing.expect(t, ok && math.classify(f) == math.Float_Class.NaN, "expected f64(nan), fully consumed") - f, ok = strconv.parse_f64("nAN") - testing.expect(t, ok && math.classify(f) == math.Float_Class.NaN, "expected f64(nan), fully consumed") - } - { - f, ok := strconv.parse_f32("1.2") - testing.expect(t, ok && f == 1.2, "expected f32(1.2), fully consumed") - - f, ok = strconv.parse_f32("1.2a") - testing.expect(t, !ok && f == 1.2, "expected f32(1.2), partially consumed") - - f, ok = strconv.parse_f32("inf") - testing.expect(t, ok && math.classify(f) == math.Float_Class.Inf, "expected f32(+inf), fully consumed") - f, ok = strconv.parse_f32("+inf") - testing.expect(t, ok && math.classify(f) == math.Float_Class.Inf, "expected f32(+inf), fully consumed") - f, ok = strconv.parse_f32("-inf") - testing.expect(t, ok && math.classify(f) == math.Float_Class.Neg_Inf, "expected f32(-inf), fully consumed") - f, ok = strconv.parse_f32("inFinity") - testing.expect(t, !ok && math.classify(f) == math.Float_Class.Inf, "expected f32(+inf), partially consumed") - f, ok = strconv.parse_f32("+InFinity") - testing.expect(t, !ok && math.classify(f) == math.Float_Class.Inf, "expected f32(+inf), partially consumed") - f, ok = strconv.parse_f32("-InfiniTy") - testing.expect(t, !ok && math.classify(f) == math.Float_Class.Neg_Inf, "expected f32(-inf), partially consumed") - f, ok = strconv.parse_f32("nan") - testing.expect(t, ok && math.classify(f) == math.Float_Class.NaN, "expected f32(nan), fully consumed") - f, ok = strconv.parse_f32("nAN") - testing.expect(t, ok && math.classify(f) == math.Float_Class.NaN, "expected f32(nan), fully consumed") - } -} \ No newline at end of file From 1fc6ff91b205836fad0fb30b36bfd042e381aa19 Mon Sep 17 00:00:00 2001 From: Feoramund <161657516+Feoramund@users.noreply.github.com> Date: Tue, 4 Jun 2024 15:17:57 -0400 Subject: [PATCH 4/5] Add test for `infinity` with trailing characters --- tests/core/strconv/test_core_strconv.odin | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/core/strconv/test_core_strconv.odin b/tests/core/strconv/test_core_strconv.odin index d33ac06b6..ed4adaf01 100644 --- a/tests/core/strconv/test_core_strconv.odin +++ b/tests/core/strconv/test_core_strconv.odin @@ -136,4 +136,19 @@ test_infinity :: proc(t: ^testing.T) { testing.expect_value(t, ok, true) testing.expect_value(t, math.classify(f), math.Float_Class.Inf) } + + // Explicitly check how trailing characters are handled. + s = "infinityyyy" + f, ok := strconv.parse_f64(s, &n) + testing.expect_value(t, f, pos_inf) + testing.expect_value(t, n, 8) + testing.expect_value(t, ok, false) + testing.expect_value(t, math.classify(f), math.Float_Class.Inf) + + s = "inflippity" + f, ok = strconv.parse_f64(s, &n) + testing.expect_value(t, f, pos_inf) + testing.expect_value(t, n, 3) + testing.expect_value(t, ok, false) + testing.expect_value(t, math.classify(f), math.Float_Class.Inf) } From 25feff3eb4c3845e6da4e8e56010301e316b2dd9 Mon Sep 17 00:00:00 2001 From: Feoramund <161657516+Feoramund@users.noreply.github.com> Date: Tue, 4 Jun 2024 18:50:08 -0400 Subject: [PATCH 5/5] Permit parsing of incomplete `infinity` but do not return true To clarify, `parse_f64` will indeed take `infi` to mean `+Inf` and return that as the value, but it will not return `ok = true`. It treats it as `inf` followed by any other trailing character. `parse_f64_prefix` is the lenient one which will return true so long as it finds some meaningful value. --- core/strconv/strconv.odin | 10 ++++- tests/core/strconv/test_core_strconv.odin | 45 +++++++++-------------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/core/strconv/strconv.odin b/core/strconv/strconv.odin index 990b2be2f..60b4b4e2e 100644 --- a/core/strconv/strconv.odin +++ b/core/strconv/strconv.odin @@ -879,9 +879,15 @@ parse_f64_prefix :: proc(str: string) -> (value: f64, nr: int, ok: bool) { fallthrough case 'i', 'I': m := common_prefix_len_ignore_case(s, "infinity") - if m == 3 || m == 8 { // "inf" or "infinity" + if 3 <= m && m < 9 { // "inf" to "infinity" f = 0h7ff00000_00000000 if sign == 1 else 0hfff00000_00000000 - n = nsign + m + if m == 8 { + // We only count the entire prefix if it is precisely "infinity". + n = nsign + m + } else { + // The string was either only "inf" or incomplete. + n = nsign + 3 + } ok = true return } diff --git a/tests/core/strconv/test_core_strconv.odin b/tests/core/strconv/test_core_strconv.odin index ed4adaf01..6b70654cc 100644 --- a/tests/core/strconv/test_core_strconv.odin +++ b/tests/core/strconv/test_core_strconv.odin @@ -62,18 +62,15 @@ test_infinity :: proc(t: ^testing.T) { n: int s := "infinity" - for i in 1 ..< len(s) + 1 { + for i in 0 ..< len(s) + 1 { ss := s[:i] f, ok := strconv.parse_f64(ss, &n) - if i == 3 { // "inf" + if i >= 3 { // "inf" .. "infinity" + expected_n := 8 if i == 8 else 3 + expected_ok := i == 3 || i == 8 testing.expect_value(t, f, pos_inf) - testing.expect_value(t, n, 3) - testing.expect_value(t, ok, true) - testing.expect_value(t, math.classify(f), math.Float_Class.Inf) - } else if i == 8 { // "infinity" - testing.expect_value(t, f, pos_inf) - testing.expect_value(t, n, 8) - testing.expect_value(t, ok, true) + testing.expect_value(t, n, expected_n) + testing.expect_value(t, ok, expected_ok) testing.expect_value(t, math.classify(f), math.Float_Class.Inf) } else { // invalid substring testing.expect_value(t, f, 0) @@ -84,18 +81,15 @@ test_infinity :: proc(t: ^testing.T) { } s = "+infinity" - for i in 1 ..< len(s) + 1 { + for i in 0 ..< len(s) + 1 { ss := s[:i] f, ok := strconv.parse_f64(ss, &n) - if i == 4 { // "+inf" + if i >= 4 { // "+inf" .. "+infinity" + expected_n := 9 if i == 9 else 4 + expected_ok := i == 4 || i == 9 testing.expect_value(t, f, pos_inf) - testing.expect_value(t, n, 4) - testing.expect_value(t, ok, true) - testing.expect_value(t, math.classify(f), math.Float_Class.Inf) - } else if i == 9 { // "+infinity" - testing.expect_value(t, f, pos_inf) - testing.expect_value(t, n, 9) - testing.expect_value(t, ok, true) + testing.expect_value(t, n, expected_n) + testing.expect_value(t, ok, expected_ok) testing.expect_value(t, math.classify(f), math.Float_Class.Inf) } else { // invalid substring testing.expect_value(t, f, 0) @@ -106,18 +100,15 @@ test_infinity :: proc(t: ^testing.T) { } s = "-infinity" - for i in 1 ..< len(s) + 1 { + for i in 0 ..< len(s) + 1 { ss := s[:i] f, ok := strconv.parse_f64(ss, &n) - if i == 4 { // "-inf" + if i >= 4 { // "-inf" .. "infinity" + expected_n := 9 if i == 9 else 4 + expected_ok := i == 4 || i == 9 testing.expect_value(t, f, neg_inf) - testing.expect_value(t, n, 4) - testing.expect_value(t, ok, true) - testing.expect_value(t, math.classify(f), math.Float_Class.Neg_Inf) - } else if i == 9 { // "-infinity" - testing.expect_value(t, f, neg_inf) - testing.expect_value(t, n, 9) - testing.expect_value(t, ok, true) + testing.expect_value(t, n, expected_n) + testing.expect_value(t, ok, expected_ok) testing.expect_value(t, math.classify(f), math.Float_Class.Neg_Inf) } else { // invalid substring testing.expect_value(t, f, 0)