[m-rev.] for review: overflow in string-to-float conversions

Julien Fischer jfischer at opturion.com
Mon Dec 22 12:36:40 AEDT 2014


For review by anyone.

(The Erlang backend will need to be looked into separately.)

---------------

Overflow in string-to-float conversions.

The behaviour of string.to_float/2 is not currently consistent between backends
when the conversion overflows.  The C and Java backends return infinity or
-infinity, while the C# backend fails.  Fix this by specifying that
string.to_float/2 should return infinity or -infinity on the conversion
overflowing and modify the C# implementation to do this.

library/string.m:
 	Specify what happens when string.to_float/2 overflows.

 	Address the XXX in the C# implementation of string.to_float/2:
 	we need to catch System.OverflowException.  If we do catch
 	it then return +/- infinity as appropriate.

 	Fix a typo: s/Formating/Formatting/

tests/hard_coded/Mmakefile:
tests/hard_coded/string_to_float_overflow.{m,exp}:
 	Add a test for string-to-float conversion overflow.

Julien.

diff --git a/library/string.m b/library/string.m
index ed8b7ce..d41147e 100644
--- a/library/string.m
+++ b/library/string.m
@@ -1098,13 +1098,15 @@
      %
  :- func det_base_string_to_int(int, string) = int.

-    % Convert a string to a float. If the string is not a syntactically
-    % correct float literal, to_float fails.
+    % Convert a string to a float, returning infinity or -infinity if the
+    % conversion overflows.  Fails if the string is not a syntactically correct
+    % float literal.
      %
  :- pred to_float(string::in, float::out) is semidet.

-    % Convert a string to a float. Throws an exception if the string is not
-    % a syntactically correct float literal.
+    % Convert a string to a float, returning infinity or -infinity if the
+    % conversion overflows.  Fails if the string is not a syntactically correct
+    % float literal.
      %
  :- func det_to_float(string) = float.

@@ -4684,7 +4686,7 @@ string.foldr_substring(Closure, String, Start, Count, !Acc) :-

  %---------------------------------------------------------------------------%
  %
-% Formating tables.
+% Formatting tables.
  %
  % Currently, string.format_table simply assumes each code point occupies
  % a single column in a fixed-width output device. Thus the output will
@@ -4948,21 +4950,25 @@ accumulate_negative_int(Base, Char, N0, N) :-
      FloatVal = 0.0;     // FloatVal must be initialized to suppress
                          // error messages when the predicate fails.

-    // leading or trailing whitespace is not allowed
+    // Leading or trailing whitespace is not allowed.
      if (FloatString.Length == 0 ||
          System.Char.IsWhiteSpace(FloatString, 0) ||
          System.Char.IsWhiteSpace(FloatString, FloatString.Length - 1))
      {
          SUCCESS_INDICATOR = false;
      } else {
-        /*
-        ** XXX should we also catch System.OverflowException?
-        */
          try {
              FloatVal = System.Convert.ToDouble(FloatString);
              SUCCESS_INDICATOR = true;
          } catch (System.FormatException) {
              SUCCESS_INDICATOR = false;
+        } catch (System.OverflowException) {
+            if (FloatString[0] == '-') {
+                FloatVal = System.Double.NegativeInfinity;
+            } else {
+                FloatVal = System.Double.PositiveInfinity;
+            }
+            SUCCESS_INDICATOR = true;
          }
      }
  }").
@@ -4973,7 +4979,7 @@ accumulate_negative_int(Base, Char, N0, N) :-
      FloatVal = 0.0;     // FloatVal must be initialized to suppress
                          // error messages when the predicate fails.

-    // leading or trailing whitespace is not allowed
+    // Leading or trailing whitespace is not allowed.
      if (FloatString.length() == 0 || FloatString.trim() != FloatString) {
          SUCCESS_INDICATOR = false;
      } else {
diff --git a/tests/hard_coded/Mmakefile b/tests/hard_coded/Mmakefile
index 6dc9480..effdd3e 100644
--- a/tests/hard_coded/Mmakefile
+++ b/tests/hard_coded/Mmakefile
@@ -301,6 +301,7 @@ ORDINARY_PROGS=	\
  	string_switch \
  	string_switch2 \
  	string_switch3 \
+	string_to_float_overflow \
  	string_various \
  	sv_nested_closures \
  	sv_record_update \
diff --git a/tests/hard_coded/string_to_float_overflow.exp b/tests/hard_coded/string_to_float_overflow.exp
new file mode 100644
index 0000000..cde59ce
--- /dev/null
+++ b/tests/hard_coded/string_to_float_overflow.exp
@@ -0,0 +1,2 @@
+infinity
+-infinity
diff --git a/tests/hard_coded/string_to_float_overflow.m b/tests/hard_coded/string_to_float_overflow.m
new file mode 100644
index 0000000..1fcf87c
--- /dev/null
+++ b/tests/hard_coded/string_to_float_overflow.m
@@ -0,0 +1,22 @@
+% Test the overflow behaviour of string.to_float/2.
+
+:- module string_to_float_overflow.
+:- interface.
+
+:- import_module io.
+
+:- pred main(io::di, io::uo) is det.
+
+:- implementation.
+
+:- import_module string.
+
+main(!IO) :-
+    ( if string.to_float("4123e358", Float1)
+    then io.print_line(Float1, !IO)
+    else io.print_line("conversion (+) failed", !IO)
+    ),
+    ( if string.to_float("-4123e358", Float2)
+    then io.print_line(Float2, !IO)
+    else io.print_line("conversion (-) failed", !IO)
+    ).



More information about the reviews mailing list