[m-rev.] for review: standardise formatting of float special values

Julien Fischer jfischer at opturion.com
Fri Sep 12 17:30:07 AEST 2014


For review by anyone.

Standardise formatting of float special values.

The string representation of float special values is currently dependent on the
target language and, for C grades, the underlying platform.  As a result,
values like NaN and Infinity are being printed or converted into strings in
variety of ways.  (See, for example bug #348 in Mantis.)

This diff changes float->string conversion to detect float special values and
use a consistent string representation for them.

library/string.m:
library/io.m:
runtime/mercury_float.c:
     Check if a float is not-a-number or of infinite magnitude when converting
     floats into strings.  If so, then use a standard representation of the
     value: "nan" for not-a-number values, and "infinity" / "-infinity" for
     values of infinite magnitude.
     For uppercase conversion specifiers in format strings, we output the above
     strings in uppercase (e.g. "NAN", "INFINITY").

library/float.m:
     Add is_infinite/1 and is_nan_or_infinite/1 as synonyms for is_inf/1 and
     is_nan_or_inf/1 respectively.

     Group float classification predicates into their own section.  Upcoming
     change will add more of them.

NEWS:
     Announce the above changes.

tests/general/string_format_special_floats.m:
tests/general/string_format_special_floats.exp:
     Modify this test to check that we actually print float special values as
     above.

tests/hard_coded/write_float_special.exp:
     Conform to the above change.

tests/EXPECT_FAIL_TESTS.all_grades:
     Do not expect the write_float_special test case to fail any more since
     the expected outputs should be the same on every platform.

Julien.

diff --git a/NEWS b/NEWS
index 0af8eec..b444c64 100644
--- a/NEWS
+++ b/NEWS
@@ -62,6 +62,9 @@ Changes to the Mercury standard library:
    NOTE: existing code that calls char.digit_to_int/2 assuming that it will
    only succeed for decimal digits (0-9) may be broken.

+* Float special values, NaNs and Infinities, are now converted to strings in
+  a way that is backend and grade-independent.  (Bug #348)
+
  Changes to the Mercury compiler:

  * We have fixed a long-standing bug causing crashes in deep profiling
diff --git a/library/float.m b/library/float.m
index ddcd620..4b7b0c2 100644
--- a/library/float.m
+++ b/library/float.m
@@ -160,19 +160,32 @@
  	%
  :- func hash(float) = int.

-	% Is the floating point number not a number or infinite?
+%----------------------------------------------------------------------------%
+%
+% Classification
+%
+
+    % Is the floating point number of infinite magnitude?
+    %
+:- pred is_infinite(float::in) is semidet.
+
+	% Synonym for the above.
  	%
-:- pred is_nan_or_inf(float::in) is semidet.
+:- pred is_inf(float::in) is semidet.

-	% Is the floating point number not a number?
+    % Is the floating point number not a number?
  	%
  :- pred is_nan(float::in) is semidet.

-	% Is the floating point number infinite?
+	% Is the floating point number not a number or of infinite magnitude?
  	%
-:- pred is_inf(float::in) is semidet.
+:- pred is_nan_or_infinite(float::in) is semidet.

-%---------------------------------------------------------------------------%
+    % Synonym for is_nan_or_inf/1.
+    %
+:- pred is_nan_or_inf(float::in) is semidet.
+
+%----------------------------------------------------------------------------%
  %
  % System constants
  %
@@ -628,6 +641,9 @@ float.multiply_by_pow(Scale0, Base, Exp) = Result :-

  %---------------------------------------------------------------------------%

+is_nan_or_infinite(Float) :-
+    is_nan_or_inf(Float).
+
  is_nan_or_inf(Float) :-
  	( is_nan(Float)
  	; is_inf(Float)
@@ -668,6 +684,8 @@ is_nan_or_inf(Float) :-
      SUCCESS_INDICATOR = false
  ").

+is_infinite(F) :-
+   is_inf(F).

  :- pragma foreign_proc("C",
  	is_inf(Flt::in),
diff --git a/library/io.m b/library/io.m
index 0fde26c..1a6c9db 100644
--- a/library/io.m
+++ b/library/io.m
@@ -7986,7 +7986,19 @@ io.write_bitmap(Bitmap, Start, NumBytes, !IO) :-
      io.write_float(Val::in, _IO0::di, _IO::uo),
      [may_call_mercury, promise_pure, thread_safe, tabled_for_io, terminates],
  "
-    io.mercury_current_text_output.get().write_or_throw(Double.toString(Val));
+    io.MR_TextOutputFile stream = io.mercury_current_text_output.get();
+
+    if (Double.isNaN(Val)) {
+        stream.write_or_throw(""nan"");
+    } else if (Double.isInfinite(Val)) {
+        if (Val < 0.0) {
+            stream.write_or_throw(""-infinity"");
+        } else {
+            stream.write_or_throw(""infinity"");
+        }
+    } else {
+        stream.write_or_throw(Double.toString(Val));
+    }
  ").

  :- pragma foreign_proc("Java",
diff --git a/library/string.m b/library/string.m
index 89442bd..9693bf9 100644
--- a/library/string.m
+++ b/library/string.m
@@ -3001,7 +3001,10 @@ specifier_to_string(spec_conv(Flags, Width, Prec, Spec)) = String :-
      ;
          % Valid float conversion specifiers.
          Spec = e(Float),
-        ( using_sprintf ->
+        (
+            not is_nan_or_infinite(Float),
+            using_sprintf
+        ->
              FormatStr = make_format(Flags, Width, Prec, "", "e"),
              String = native_format_float(FormatStr, Float)
          ;
@@ -3010,7 +3013,10 @@ specifier_to_string(spec_conv(Flags, Width, Prec, Spec)) = String :-
          )
      ;
          Spec = cE(Float),
-        ( using_sprintf ->
+        (
+            not is_nan_or_infinite(Float),
+            using_sprintf
+        ->
              FormatStr = make_format(Flags, Width, Prec, "", "E"),
              String = native_format_float(FormatStr, Float)
          ;
@@ -3019,7 +3025,10 @@ specifier_to_string(spec_conv(Flags, Width, Prec, Spec)) = String :-
          )
      ;
          Spec = f(Float),
-        ( using_sprintf ->
+        (
+            not is_nan_or_infinite(Float),
+            using_sprintf
+        ->
              FormatStr = make_format(Flags, Width, Prec, "", "f"),
              String = native_format_float(FormatStr, Float)
          ;
@@ -3028,7 +3037,10 @@ specifier_to_string(spec_conv(Flags, Width, Prec, Spec)) = String :-
          )
      ;
          Spec = cF(Float),
-        ( using_sprintf ->
+        (
+            not is_nan_or_infinite(Float),
+            using_sprintf
+        ->
              FormatStr = make_format(Flags, Width, Prec, "", "F"),
              String = native_format_float(FormatStr, Float)
          ;
@@ -3037,7 +3049,10 @@ specifier_to_string(spec_conv(Flags, Width, Prec, Spec)) = String :-
          )
      ;
          Spec = g(Float),
-        ( using_sprintf ->
+        (
+            not is_nan_or_infinite(Float),
+            using_sprintf
+        ->
              FormatStr = make_format(Flags, Width, Prec, "", "g"),
              String = native_format_float(FormatStr, Float)
          ;
@@ -3046,7 +3061,10 @@ specifier_to_string(spec_conv(Flags, Width, Prec, Spec)) = String :-
          )
      ;
          Spec = cG(Float),
-        ( using_sprintf ->
+        (
+            not is_nan_or_infinite(Float),
+            using_sprintf
+        ->
              FormatStr = make_format(Flags, Width, Prec, "", "G"),
              String = native_format_float(FormatStr, Float)
          ;
@@ -3510,8 +3528,8 @@ format_unsigned_int(Flags, Width, Prec, Base, Int, IsTypeP, Prefix) = String :-
  format_float(Flags, SpecCase, Width, Prec, Float) = NewFloat :-
      ( is_nan(Float) ->
          SignedStr = format_nan(SpecCase)
-    ; is_inf(Float) ->
-        SignedStr = format_inf(Float, SpecCase)
+    ; is_infinite(Float) ->
+        SignedStr = format_infinity(Float, SpecCase)
      ;
          % Determine absolute value of string.
          Abs = abs(Float),
@@ -3565,8 +3583,8 @@ format_scientific_number_g(Flags, SpecCase, Width, Prec, Float, E)
          = NewFloat :-
      ( is_nan(Float) ->
          SignedStr = format_nan(SpecCase)
-    ; is_inf(Float) ->
-        SignedStr = format_inf(Float, SpecCase)
+    ; is_infinite(Float) ->
+        SignedStr = format_infinity(Float, SpecCase)
      ;
          % Determine absolute value of string.
          Abs = abs(Float),
@@ -3612,8 +3630,8 @@ format_scientific_number_g(Flags, SpecCase, Width, Prec, Float, E)
  format_scientific_number(Flags, SpecCase, Width, Prec, Float, E) = NewFloat :-
      ( is_nan(Float) ->
          SignedStr = format_nan(SpecCase)
-    ; is_inf(Float) ->
-        SignedStr = format_inf(Float, SpecCase)
+    ; is_infinite(Float) ->
+        SignedStr = format_infinity(Float, SpecCase)
      ;
          % Determine absolute value of string.
          Abs = abs(Float),
@@ -4148,15 +4166,15 @@ is_exponent('E').
  format_nan(spec_is_capital) = "NAN".
  format_nan(spec_is_not_capital) = "nan".

-:- func format_inf(float, spec_case) = string.
+:- func format_infinity(float, spec_case) = string.

-format_inf(F, SpecCase) = String :-
+format_infinity(F, SpecCase) = String :-
      (
          SpecCase = spec_is_capital,
-        String = ( if F < 0.0 then "-INF" else "INF" )
+        String = ( if F < 0.0 then "-INFINITY" else "INFINITY" )
      ;
          SpecCase = spec_is_not_capital,
-        String = ( if F < 0.0 then "-inf" else "inf" )
+        String = ( if F < 0.0 then "-infinity" else "infinity" )
      ).

  %-----------------------------------------------------------------------------%
@@ -4203,9 +4221,15 @@ string.from_float(Flt) = string.float_to_string(Flt).
      [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
          does_not_affect_liveness, no_sharing],
  "
-    Str = Flt.ToString(""R"");
+    if (System.Double.IsNaN(Flt)) {
+        Str = ""nan"";
+    } else if (System.Double.IsPositiveInfinity(Flt)) {
+        Str = ""infinity"";
+    } else if (System.Double.IsNegativeInfinity(Flt)) {
+        Str = ""-infinity"";
+    } else {

-    if (!System.Double.IsNaN(Flt) && !System.Double.IsInfinity(Flt)) {
+        Str = Flt.ToString(""R"");

          /* Append '.0' if there is no 'e' or '.' in the string. */
          bool contains = false;
@@ -4226,7 +4250,17 @@ string.from_float(Flt) = string.float_to_string(Flt).
      [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
          does_not_affect_liveness, no_sharing],
  "
-    Str = java.lang.Double.toString(Flt);
+    if (Double.isNaN(Flt)) {
+        Str = ""nan"";
+    } else if (Double.isInfinite(Flt)) {
+        if (Flt < 0.0) {
+            Str = ""-infinity"";
+        } else {
+            Str = ""infinity"";
+        }
+    } else {
+        Str = java.lang.Double.toString(Flt);
+    }
  ").

  % XXX This implementation has problems when the mantissa cannot fit in an int.
diff --git a/runtime/mercury_float.c b/runtime/mercury_float.c
index 19a86a5..90ecfc4 100644
--- a/runtime/mercury_float.c
+++ b/runtime/mercury_float.c
@@ -58,7 +58,7 @@ MR_hash_float(MR_Float f)
  /*
  ** MR_sprintf_float(buf, f)
  **
-** fills buff with the string representation of the float, f, such that
+** Fills buf with the string representation of the float, f, such that
  ** the string representation has enough precision to represent the
  ** float, f.
  **
@@ -71,6 +71,20 @@ MR_sprintf_float(char *buf, MR_Float f)
      MR_Float round_trip = 0.0;
      int      i = MR_FLT_MIN_PRECISION;

+    if (MR_is_nan(f)) {
+        strcpy(buf, "nan");
+        return;
+    }
+
+    if (MR_is_inf(f)) {
+        if (f < 0) {
+            strcpy(buf, "-infinity");
+        } else {
+            strcpy(buf, "infinity");
+        }
+        return;
+    }
+
      /*
      ** Print the float at increasing precisions until the float
      ** is round-trippable.
@@ -90,13 +104,6 @@ MR_sprintf_float(char *buf, MR_Float f)
      } while (round_trip != f);

      /*
-    ** Do not append ".0" to nan or (-)inf.
-    */
-    if (MR_is_nan(f) || MR_is_inf(f)) {
-        return;
-    }
-
-    /*
      ** Append ".0" if there is no "e" or "." in the string.
      */
      while (1) {
diff --git a/tests/EXPECT_FAIL_TESTS.all_grades b/tests/EXPECT_FAIL_TESTS.all_grades
index aa762c5..febbde7 100644
--- a/tests/EXPECT_FAIL_TESTS.all_grades
+++ b/tests/EXPECT_FAIL_TESTS.all_grades
@@ -1,6 +1,5 @@
  debugger/declarative/condition_bug
  hard_coded/typeclasses/complicated_constraint
-hard_coded/write_float_special
  valid/bug85
  valid/constraint_prop_bug
  valid/csharp_hello
diff --git a/tests/general/string_format_special_floats.exp b/tests/general/string_format_special_floats.exp
index 640718a..b75d38b 100644
--- a/tests/general/string_format_special_floats.exp
+++ b/tests/general/string_format_special_floats.exp
@@ -1,21 +1,21 @@
  Plus Infinity:
-                  %e: success
-                  %f: success
-                  %g: success
-                  %E: success
-                  %F: success
-                  %G: success
+                  %e: infinity
+                  %f: infinity
+                  %g: infinity
+                  %E: INFINITY
+                  %F: INFINITY
+                  %G: INFINITY
  Minus Infinity:
-                  %e: success
-                  %f: success
-                  %g: success
-                  %E: success
-                  %F: success
-                  %G: success
+                  %e: -infinity
+                  %f: -infinity
+                  %g: -infinity
+                  %E: -INFINITY
+                  %F: -INFINITY
+                  %G: -INFINITY
  Not a number:
-                  %e: success
-                  %f: success
-                  %g: success
-                  %E: success
-                  %F: success
-                  %G: success
+                  %e: nan
+                  %f: nan
+                  %g: nan
+                  %E: NAN
+                  %F: NAN
+                  %G: NAN
diff --git a/tests/general/string_format_special_floats.m b/tests/general/string_format_special_floats.m
index e9d89ef..d2c5cd1 100644
--- a/tests/general/string_format_special_floats.m
+++ b/tests/general/string_format_special_floats.m
@@ -23,59 +23,22 @@ main(!IO) :-
      Inf = (max + max),
      TestSpecs = ["%e", "%f", "%g", "%E", "%F", "%G"],
      io.write_string("Plus Infinity:\n", !IO),
-    list.foldl(test_floats(is_plus_infinity, [Inf]), TestSpecs, !IO),
+    list.foldl(test_floats([Inf]), TestSpecs, !IO),
      io.write_string("Minus Infinity:\n", !IO),
-    list.foldl(test_floats(is_minus_infinity, [-Inf]), TestSpecs, !IO),
+    list.foldl(test_floats([-Inf]), TestSpecs, !IO),
      io.write_string("Not a number:\n", !IO),
-    list.foldl(test_floats(is_nan, [0.0 * Inf]), TestSpecs, !IO).
+    list.foldl(test_floats([0.0 * Inf]), TestSpecs, !IO).

-:- pred test_floats(pred(string)::in(pred(in) is semidet), list(float)::in,
-    string::in, io::di, io::uo) is det.
+:- pred test_floats(list(float)::in, string::in, io::di, io::uo) is det.

-test_floats(IsValid, Floats, FormatString, !IO) :-
-    list.foldl(test_float(FormatString, IsValid), Floats, !IO).
+test_floats(Floats, FormatString, !IO) :-
+    list.foldl(test_float(FormatString), Floats, !IO).

-:- pred test_float(string::in, pred(string)::in(pred(in) is semidet),
-    float::in, io::di, io::uo) is det.
+:- pred test_float(string::in, float::in, io::di, io::uo) is det.

-test_float(FormatString, IsValid, Float, !IO) :-
+test_float(FormatString, Float, !IO) :-
      FloatString = string.format(FormatString, [f(Float)]),
-    io.format("%20s: ", [s(FormatString)], !IO),
-    ( IsValid(FloatString) ->
-        io.write_string("success\n", !IO)
-    ;
-        io.write_string("failure '" ++ FloatString ++ "'\n", !IO)
-    ).
-
-:- pred is_plus_infinity(string::in) is semidet.
-
-is_plus_infinity(String) :-
-    LowerCaseString = string.to_lower(String),
-    ( LowerCaseString = "infinity"
-    ; LowerCaseString = "inf"
-    ).
-
-:- pred is_minus_infinity(string::in) is semidet.
-
-is_minus_infinity(String) :-
-    LowerCaseString = string.to_lower(String),
-    ( LowerCaseString = "-infinity"
-    ; LowerCaseString = "-inf"
-    ).
-
-:- pred is_nan(string::in) is semidet.
-
-is_nan(String) :-
-    LowerCaseString = string.to_lower(String),
-    ( LowerCaseString = "nan"
-    % XXX Actually, it makes no sense to put a minus sign on a NaN,
-    %     since NaNs aren't signed.  However, the printf() function in
-    %     some C libraries (in particular, the one for Solaris 2.7)
-    %     do that.  Arguably that's a bug, but we can't do much about
-    %     bugs in the Solaris C library, so we don't want to report a
-    %     test case failure for that.  Hence we allow -NaN here.
-    ; LowerCaseString = "-nan"
-    ).
+    io.format("%20s: %s\n", [s(FormatString), s(FloatString)], !IO).

  %------------------------------------------------------------------------------%
  %------------------------------------------------------------------------------%
diff --git a/tests/hard_coded/write_float_special.exp b/tests/hard_coded/write_float_special.exp
index cb27352..ad476d5 100644
--- a/tests/hard_coded/write_float_special.exp
+++ b/tests/hard_coded/write_float_special.exp
@@ -1,3 +1,3 @@
-Inf: inf
--Inf: -inf
+Inf: infinity
+-Inf: -infinity
  NaN: nan
diff --git a/tests/hard_coded/write_float_special.exp2 b/tests/hard_coded/write_float_special.exp2
deleted file mode 100644
index ad476d5..0000000
--- a/tests/hard_coded/write_float_special.exp2
+++ /dev/null
@@ -1,3 +0,0 @@
-Inf: infinity
--Inf: -infinity
-NaN: nan
diff --git a/tests/hard_coded/write_float_special.exp3 b/tests/hard_coded/write_float_special.exp3
deleted file mode 100644
index 207df85..0000000
--- a/tests/hard_coded/write_float_special.exp3
+++ /dev/null
@@ -1,3 +0,0 @@
-Inf: Infinity
--Inf: -Infinity
-NaN: NaN



More information about the reviews mailing list