Change int.fromEnvironment in VM to use same code as int.parse.

runtime/lib/integers.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/bootstrap_natives.h"
 
 #include "include/dart_api.h"
 #include "vm/bigint_operations.h"
 #include "vm/dart_entry.h"
 #include "vm/dart_api_impl.h"
@@ skipping 173 matching lines @@
   ASSERT(CheckInteger(left));
   ASSERT(CheckInteger(right));
   if (FLAG_trace_intrinsified_natives) {
     OS::Print("Integer_equalToInteger %s == %s\n",
         left.ToCString(), right.ToCString());
   }
   return Bool::Get(left.CompareWith(right) == 0).raw();
 }
 
 
-DEFINE_NATIVE_ENTRY(Integer_parse, 1) {
-  GET_NON_NULL_NATIVE_ARGUMENT(String, value, arguments->NativeArgAt(0));
+static RawInteger* ParseInteger(const String& value) {
+  // Used by both Integer_parse and Integer_fromEnvironment.
   if (value.IsOneByteString()) {
     // Quick conversion for unpadded integers in strings.
     const intptr_t len = value.Length();
     if (len > 0) {
       const char* cstr = value.ToCString();
       ASSERT(cstr != NULL);
       char* p_end = NULL;
       const int64_t int_value = strtoll(cstr, &p_end, 10);
       if (p_end == (cstr + len)) {
         if ((int_value != LLONG_MIN) && (int_value != LLONG_MAX)) {
@@ skipping 12 matching lines @@
                              &is_positive,
                              &int_string)) {
     if (is_positive) {
       return Integer::New(*int_string);
     }
     String& temp = String::Handle();
     temp = String::Concat(Symbols::Dash(), *int_string);
     return Integer::New(temp);
   }
 
-  return Object::null();
+  return Integer::null();
 }
 
 
+DEFINE_NATIVE_ENTRY(Integer_parse, 1) {
+  GET_NON_NULL_NATIVE_ARGUMENT(String, value, arguments->NativeArgAt(0));
+  return ParseInteger(value);
+}
+
+
 DEFINE_NATIVE_ENTRY(Integer_fromEnvironment, 3) {
   GET_NON_NULL_NATIVE_ARGUMENT(String, name, arguments->NativeArgAt(1));
   GET_NATIVE_ARGUMENT(Integer, default_value, arguments->NativeArgAt(2));
   // Call the embedder to supply us with the environment.
   Dart_EnvironmentCallback callback = isolate->environment_callback();
   if (callback != NULL) {
-    Dart_Handle result = callback(Api::NewHandle(isolate, name.raw()));
-    if (Dart_IsError(result)) {
+    Dart_Handle response = callback(Api::NewHandle(isolate, name.raw()));
+    if (Dart_IsString(response)) {
+      const String& value = String::Cast(
+          Object::Handle(isolate, Api::UnwrapHandle(response)));
+      const Integer& result = Integer::Handle(ParseInteger(value));
+      if (!result.IsNull()) {
+        if (result.IsSmi()) return result.raw();

[Ivan Posva, 2013/10/31 16:08:18]

{
  return result.raw();
}

+        return Integer::NewCanonical(String::Handle(
+            String::New(result.ToCString())));

[Lasse Reichstein Nielsen, 2013/10/31 13:45:24]

Ivan, we can't decide if this canonicalization is still necessary.
If not, we can just return result.raw in all cases.

[Ivan Posva, 2013/10/31 16:08:18]

Yes, it is necessary. You could of course do

return result.CheckAndCanonicalize();

to avoid parsing again.

+      }
+    } else if (Dart_IsError(response)) {
       const Object& error =
-          Object::Handle(isolate, Api::UnwrapHandle(result));
+          Object::Handle(isolate, Api::UnwrapHandle(response));
       Exceptions::ThrowArgumentError(
           String::Handle(
               String::New(Error::Cast(error).ToErrorCString())));
-    } else if (Dart_IsString(result)) {
-      uint8_t* digits;
-      intptr_t digits_len;
-      Dart_StringToUTF8(result, &digits, &digits_len);
-      if (digits_len > 0) {
-        // Check for valid integer literal before constructing integer object.
-        // Skip leading minus if present.
-        if (digits[0] == '-') {
-          digits++;
-          digits_len--;
-        }
-        // Check remaining string for decimal or hex-decimal literal.
-        bool is_number = true;
-        if (digits_len > 2 &&
-            digits[0] == '0' &&
-            (digits[1] == 'x' || digits[1] == 'X')) {
-          for (int i = 2; i < digits_len && is_number; i++) {
-            is_number = ('0' <= digits[i] && digits[i] <= '9') ||
-                        ('A' <= digits[i] && digits[i] <= 'F') ||
-                        ('a' <= digits[i] && digits[i] <= 'f');
-          }
-        } else {
-          for (int i = 0; i < digits_len && is_number; i++) {
-            is_number = '0' <= digits[i] && digits[i] <= '9';
-          }
-        }
-        if (digits_len > 0 && is_number) {
-          const Object& value =
-              Object::Handle(isolate, Api::UnwrapHandle(result));
-          ASSERT(value.IsString());
-          return Integer::NewCanonical(String::Cast(value));
-        }
-      }
-    } else if (!Dart_IsNull(result)) {
+    } else  if (!Dart_IsNull(response)) {
       Exceptions::ThrowArgumentError(
           String::Handle(String::New("Illegal environment value")));
     }
   }
   return default_value.raw();
 }
 
 
 // Passing true for 'silent' prevents throwing JavascriptIntegerOverflow.
 static RawInteger* ShiftOperationHelper(Token::Kind kind,
@@ skipping 170 matching lines @@
   // Use the preallocated out of memory exception to avoid calling
   // into dart code or allocating any code.
   const Instance& exception =
       Instance::Handle(isolate->object_store()->out_of_memory());
   Exceptions::Throw(exception);
   UNREACHABLE();
   return 0;
 }
 
 }  // namespace dart