Implement fromEnvironment on bool, int and String

runtime/lib/bool.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"
+#include "vm/exceptions.h"
+#include "vm/isolate.h"
+#include "vm/native_entry.h"
+#include "vm/object.h"
+#include "vm/object_store.h"
+#include "vm/symbols.h"
+
+namespace dart {
+
+DEFINE_NATIVE_ENTRY(Bool_fromEnvironment, 3) {
+  GET_NON_NULL_NATIVE_ARGUMENT(String, name, arguments->NativeArgAt(1));
+  GET_NATIVE_ARGUMENT(Bool, default_value, arguments->NativeArgAt(2));
+  // Call the embedder to supply us with the environment.
+  Dart_ConfigCallback callback = isolate->config_callback();
+  if (callback != NULL) {
+    Dart_Handle result = callback(kBoolConfig,
+                                  Api::NewHandle(isolate, name.raw()));
+    if (Dart_IsString(result)) {
+      const char *chars;
+      Dart_StringToCString(result, &chars);
+      return (strcmp("false", chars) != 0)

[Ivan Posva, 2013/10/29 21:42:59]

Only "true" is the true value in Dart, you should follow this here as well. Also
this should be handled based on the Dart_EnvironmentType in the embedder. This
code here should be expecting a boolean from the embedder.

[Søren Gjesse, 2013/10/29 22:44:39]

One question here. I was a bit puzzeled about the kXXXConfig constants for the
embedder. Should the implementation be changed to expect

  For kIntegerConfig: Null or an Integer object
  For kStringConfig: Null or a String object
  For kBoolConfig: Null or a Bool object

Then the embedder returns Null to provide the default value and otherwise the
parsing of integer and boolean strings are left to the embedder. The VM library
will not have any logic there. The embedder supplies an object of the expected
type or Null.

I had that at some point, which is also why the overloaded version of
Integer::NewCanonical is still in the CL.

[Ivan Posva, 2013/10/30 05:08:21]

I guess it would make the native API simpler and less cumbersome to use if the
callback was only ever expected to return a Dart_Null, a Dart_Error or a
Dart_String.

This was one of the open areas which I had not been happy with in my version of
the CL yet.

[Søren Gjesse, 2013/10/30 11:44:02]

Changed so that only "true" is true.

Changed the API to no longer have the type given to the callback and updated the
documentation to say return either a string or null.

+          ? Bool::True().raw() : Bool::False().raw();
+    }
+  }
+  if (default_value.IsNull()) {
+    Exceptions::ThrowArgumentError(
+        String::Handle(String::New("Environment value does not have a value")));
+  }
+  return default_value.raw();
+}
+
+}  // namespace dart