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

mrb_read_float() converts "0.3" with a small error compared to strtod() #6182

Closed
dearblue opened this issue Feb 22, 2024 · 3 comments
Closed

Comments

@dearblue
Copy link
Contributor

As far as I can tell, "0.6" and "0.7" also do not have the same value as strtod().

The reason I noticed this is mruby-marshal.
However, when I do Marshal.dump/load, the respective results for 0.6 and 0.7 seem to be equal.

% bin/mirb
mirb - Embeddable Interactive Ruby Shell

> Marshal.load(Marshal.dump(0.3)) == 0.3
 => false
> Marshal.load(Marshal.dump(0.6)) == 0.6
 => true
> Marshal.load(Marshal.dump(0.7)) == 0.7
 => true
@dearblue
Copy link
Contributor Author

For information, it seems also to have the same problem with the original, https://github.com/mattn/strtod.
Adding values in 0.1 intervals from 0.2 to 0.9 and reporting only failures, the results are shown below.

%  cc strtod.c -lm && ./a.out
CASE: .3
  ERR: .3, No error: 0
    E1 0.300000, 0.3, , 0
    E2 0.300000, 0.3, , 0
different value

CASE: .6
  ERR: .6, No error: 0
    E1 0.600000, 0.6, , 0
    E2 0.600000, 0.6, , 0
different value

CASE: .7
  ERR: .7, No error: 0
    E1 0.700000, 0.7, , 0
    E2 0.700000, 0.7, , 0
different value

CASE: 2.2250738585072010e-308
  ERR: 2.2250738585072010e-308, Result too large
    E1 0.000000, 0, , 34
    E2 0.000000, 2.22507e-308, , 34
different value

CASE: 2.2250738585072011e-308
  ERR: 2.2250738585072011e-308, Result too large
    E1 0.000000, 0, , 34
    E2 0.000000, 2.22507e-308, , 34
different value
patch for test of strtod.c
diff --git a/strtod.c b/strtod.c
index bd5378b..42bb574 100644
--- a/strtod.c
+++ b/strtod.c
@@ -148,8 +148,6 @@ test(char* str)
     char *e1, *e2;
     int x1, x2;
 
-    printf("CASE: %s\n", str);
-
     errno = 0;
     e1 = NULL;
     d1 = vim_strtod(str, &e1);
@@ -161,22 +159,29 @@ test(char* str)
     x2 = errno;
 
     if (d1 != d2 || e1 != e2 || x1 != x2) {
+        printf("CASE: %s\n", str);
 	printf("  ERR: %s, %s\n", str, strerror(errno));
 	printf("    E1 %f, %g, %s, %d\n", d1, d1, e1 ? e1 : "", x1);
 	printf("    E2 %f, %g, %s, %d\n", d2, d2, e2 ? e2 : "", x2);
        if (d1 != d2) puts("different value");
        if (e1 != e2) puts("different end position");
        if (x1 != x2) puts("different errno");
-    } else {
-	printf("  SUCCESS [%f][%s]: %s\n", d1, e1 ? e1 : "", strerror(errno));
+        printf("\n");
     }
-    printf("\n");
 }
 
     int
 main()
 {
     test(".1");
+    test(".2");
+    test(".3");
+    test(".4");
+    test(".5");
+    test(".6");
+    test(".7");
+    test(".8");
+    test(".9");
     test("  .");
     test("  1.2e3");
     test(" +1.2e3");

@matz
Copy link
Member

matz commented Feb 24, 2024

For several reasons, I recommend mruby-marshal to use mrb_read_float() instead of strtod().

  • strtod() uses locale which is fragile (for example we need to use 0,3 not 0.3 for some locale)
  • it is very hard to implement bit-to-bit equality with independent string-to-float functions

For the same reason, I recommend to use mrb_float_to_str() instead of printf().

@dearblue
Copy link
Contributor Author

Understood. Thank you very much.
I tried that too, but it's not possible.

I think it makes sense to write it in doc/limitations.md as it is now, so I'll update that one.

@dearblue dearblue closed this as not planned Won't fix, can't repro, duplicate, stale Feb 25, 2024
dearblue added a commit to dearblue/mruby that referenced this issue Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants