Implement Double.{toString, toStringAsExponential, toStringAsPrecision}

runtime/lib/double.cc

Index: runtime/lib/double.cc
diff --git a/runtime/lib/double.cc b/runtime/lib/double.cc
index 00c68e823af1033d676c8ada5744f8cd8eaa93b8..008788040597c1b0df543d44e63aefff4bb92859 100644
--- a/runtime/lib/double.cc
+++ b/runtime/lib/double.cc
@@ -189,26 +189,57 @@ DEFINE_NATIVE_ENTRY(Double_toStringAsFixed, 2) {
   const Double& arg = Double::CheckedHandle(arguments->At(0));
   GET_NATIVE_ARGUMENT(Smi, fraction_digits, arguments->At(1));
   double d = arg.value();
-  int fraction_digits_value = fraction_digits.Value();
-  String& result = String::Handle();
-  bool succeeded = DoubleToStringAsFixed(d, fraction_digits_value, result);
-  if (!succeeded) {
+  RawString* result = NULL;

[Ivan Posva, 2012/02/02 23:21:36]

You should not keep raw pointers lying around. This has potential for many
breakages in the code below because of . Also NULL is definitely the wrong thing
here, what it really converts into is a int with value 0.

String& result = String::Handle();
...
if (...) {
  result = DoubleToStringAsFixed(...);
}
if (result.IsNull()) {
...
}

[floitsch, 2012/02/05 15:28:21]

Fixed. But out of curiosity: was the current code broken, or just a potential
point of breakages in the future?
done.

[Ivan Posva, 2012/02/06 18:25:44]

It was broken since:
- Assigning NULL to a RawObject* is not what you think it is.
- Doing handle operations while holding a RawObject* can cause unexpected
behaviour.

I agree that the way the code was written it was highly unlikely to suffer from
the above points, but it is not a style we want to ever have in the VM as it
will come to haunt us later.

+  intptr_t fraction_digits_value = fraction_digits.Value();
+  if (0 <= fraction_digits_value && fraction_digits_value <= 20) {
+    result = DoubleToStringAsFixed(d, static_cast<int>(fraction_digits_value));
+  }
+  if (result == NULL) {
     GrowableArray<const Object*> args;
     args.Add(&String::ZoneHandle(String::New(
         "Illegal arguments to double.toStringAsFixed")));
     Exceptions::ThrowByType(Exceptions::kIllegalArgument, args);
   }
-  arguments->SetReturn(result);
+  arguments->SetReturn(String::Handle(result));
 }
 
 
 DEFINE_NATIVE_ENTRY(Double_toStringAsExponential, 2) {
-  UNIMPLEMENTED();
+  const Double& arg = Double::CheckedHandle(arguments->At(0));
+  GET_NATIVE_ARGUMENT(Smi, fraction_digits, arguments->At(1));
+  double d = arg.value();
+  RawString* result = NULL;

[Ivan Posva, 2012/02/02 23:21:36]

ditto

[floitsch, 2012/02/05 15:28:21]

Done.

+  intptr_t fraction_digits_value = fraction_digits.Value();
+  if (-1 <= fraction_digits_value && fraction_digits_value <= 20) {
+    result =
+        DoubleToStringAsExponential(d, static_cast<int>(fraction_digits_value));
+  }
+  if (result == NULL) {
+    GrowableArray<const Object*> args;
+    args.Add(&String::ZoneHandle(String::New(
+        "Illegal arguments to double.toStringAsExponential")));
+    Exceptions::ThrowByType(Exceptions::kIllegalArgument, args);
+  }
+  arguments->SetReturn(String::Handle(result));
 }
 
 
 DEFINE_NATIVE_ENTRY(Double_toStringAsPrecision, 2) {
-  UNIMPLEMENTED();
+  const Double& arg = Double::CheckedHandle(arguments->At(0));
+  GET_NATIVE_ARGUMENT(Smi, precision, arguments->At(1));
+  double d = arg.value();
+  RawString* result = NULL;

[Ivan Posva, 2012/02/02 23:21:36]

ditto

[floitsch, 2012/02/05 15:28:21]

Done.

+  intptr_t precision_value = precision.Value();
+  if (1 <= precision_value && precision_value <= 21) {
+    result = DoubleToStringAsPrecision(d, static_cast<int>(precision_value));
+  }
+  if (result == NULL) {
+    GrowableArray<const Object*> args;
+    args.Add(&String::ZoneHandle(String::New(
+        "Illegal arguments to double.toStringAsPrecision")));
+    Exceptions::ThrowByType(Exceptions::kIllegalArgument, args);
+  }
+  arguments->SetReturn(String::Handle(result));
 }