Check for duplicate library names

lib/runtime/dart_runtime.js

Index: lib/runtime/dart_runtime.js
diff --git a/lib/runtime/dart_runtime.js b/lib/runtime/dart_runtime.js
index 468ac5dd7aabdfe7c4bef2a83ea2643f3b83f548..e986a64ffb6e76ab779556dbefc136cdfe091db8 100644
--- a/lib/runtime/dart_runtime.js
+++ b/lib/runtime/dart_runtime.js
@@ -50,6 +50,10 @@ var dart, _js_helper, _js_primitives, dartx;
   }
   dart.dput = dput;
 
+  function throwRuntimeError(message) {
+    throw Error(message);
+  }
+
   // TODO(jmesserly): this should call noSuchMethod, not throw.
   function throwNoSuchMethod(obj, name, args, opt_func) {
     if (obj === void 0) obj = opt_func;
@@ -426,15 +430,15 @@ var dart, _js_helper, _js_primitives, dartx;
 
   /** Checks that `x` is not null or undefined. */
   function notNull(x) {
-    if (x == null) throw 'expected not-null value';
+    if (x == null) throwRuntimeError('expected not-null value');
     return x;
   }
   dart.notNull = notNull;
 
   function _typeName(type) {
-    if (type === void 0) throw "Undefined type";
+    if (type === void 0) throwRuntimeError('Undefined type');
     var name = type.name;
-    if (!name) throw 'Unexpected type: ' + type;
+    if (!name) throwRuntimeError('Unexpected type: ' + type);
     return name;
   }
 
@@ -722,7 +726,7 @@ var dart, _js_helper, _js_primitives, dartx;
     function lazyGetter() {
       // Clear the init function to detect circular initialization.
       let f = init;
-      if (f === null) throw 'circular initialization for field ' + name;
+      if (f === null) throwRuntimeError('circular initialization for field ' + name);
       init = null;
 
       // Compute and store the value.
@@ -943,8 +947,7 @@ var dart, _js_helper, _js_primitives, dartx;
   dart.map = map;
 
   function assert(condition) {
-    // TODO(jmesserly): throw assertion error.
-    if (!condition) throw 'assertion failed';
+    if (!condition) throw new core.AssertionError();
   }
   dart.assert = assert;
 
@@ -978,12 +981,12 @@ var dart, _js_helper, _js_primitives, dartx;
   /** Memoize a generic type constructor function. */
   function generic(typeConstructor) {
     let length = typeConstructor.length;
-    if (length < 1) throw Error('must have at least one generic type argument');
+    if (length < 1) throwRuntimeError('must have at least one generic type argument');
 
     let resultMap = new Map();
     function makeGenericType(/*...arguments*/) {
       if (arguments.length != length && arguments.length != 0) {
-        throw Error('requires ' + length + ' or 0 type arguments');
+        throwRuntimeError('requires ' + length + ' or 0 type arguments');
       }
       let args = slice.call(arguments);
       // TODO(leafp): This should really be core.Object for
@@ -996,7 +999,7 @@ var dart, _js_helper, _js_primitives, dartx;
       for (let i = 0; i < length; i++) {
         let arg = args[i];
         if (arg == null) {
-          throw Error('type arguments should not be null: ' + typeConstructor);
+          throwRuntimeError('type arguments should not be null: ' + typeConstructor);
         }
         let map = value;
         value = map.get(arg);
@@ -1279,32 +1282,86 @@ var dart, _js_helper, _js_primitives, dartx;
   dart.global = window || global;
   dart.JsSymbol = Symbol;
 
-  function import_(value) {
+  var libraries = {};

[Jennifer Messerly, 2015/06/05 22:45:57]

use a Map here?

[vsm, 2015/06/05 23:03:11]

Done.

+  var loadedLibraries = new Set();

[Jennifer Messerly, 2015/06/05 22:45:57]

I think the reason this exists is to track libraries that are *completed*
whereas the `libraries` can contain incomplete lazy imports?

Anyway, looks good, but maybe worth a comment?

[vsm, 2015/06/05 23:03:11]

Done.

+
+  function import_(name) {
+    let loaded = loadedLibraries.has(name);
+    let value = libraries[name];
     // TODO(vsm): Change this to a hard throw.
     // For now, we're missing some libraries.  E.g., dart:js:
     // https://github.com/dart-lang/dev_compiler/issues/168
-    if (!value) {
-      console.log('missing required module');
+    if (!loaded) {
+      console.warn('Missing required module: ' + name);
+    } else if (!value) {
+      throwRuntimeError('Library import error: ' + name)
     }
     return value;
   }
   dart.import = import_;

[Jennifer Messerly, 2015/06/05 22:45:57]

This is public for interop/REPL purposes? maybe worth a comment

[vsm, 2015/06/05 23:03:11]

Done.

+
+  function initializeLibraryStub(name) {
+    // Create the library object if necessary.
+    if (!libraries[name]) {
+      libraries[name] = {};
+    }
+    return libraries[name];
+  }
   
-  function lazyImport(value) {
-    return defineLibrary(value, {});
+  function lazyImport(name) {

[Jennifer Messerly, 2015/06/05 22:45:57]

maybe we don't need this? just rename the other one "initLibrary" ?
another way of doing it, if you really want two names:

    const lazyImport = initializeLibraryStub;

[vsm, 2015/06/05 23:03:11]

Done.

+    return initializeLibraryStub(name);
+  }
+
+  function defineLibrary(name, defaultValue) {
+    if (loadedLibraries.has(name)) {
+      throwRuntimeError('Library is already defined: ' + name);
+    }
+    var value;
+    if (defaultValue) {
+      var oldValue = libraries[name];
+      if (oldValue && oldValue != defaultValue) {
+        throwRuntimeError(
+          'Library ${name} cannot be redefined to ${defaultValue}');

[Jennifer Messerly, 2015/06/05 22:45:57]

backticks here?

[vsm, 2015/06/05 23:03:11]

Done.

+      }
+      libraries[name] = value = defaultValue;
+    } else {
+      value = initializeLibraryStub(name);
+    }
+    loadedLibraries.add(name);
+    return value;
   }
-  dart.lazyImport = lazyImport;
 
-  function defineLibrary(value, defaultValue) {
-    return value ? value : defaultValue;
+  function library(name, defaultValue, imports, lazyImports, module) {

[Jennifer Messerly, 2015/06/05 22:45:57]

for defaultValue, move last? since it's rarely supplied. (And I doubt we should
even really load dom.js ... making it a fairly rare feature. dart:html might use
but other than that.)

+    var args = [];
+    var lib = defineLibrary(name, defaultValue);
+    args.push(lib);
+    for (var i = 0; i < imports.length; ++i) {
+      lib = import_(imports[i]);
+      args.push(lib);
+    }
+    for (var i = 0; i < lazyImports.length; ++i) {
+      lib = lazyImport(lazyImports[i]);
+      args.push(lib);
+    }
+    module.apply(dart.global, args);

[Jennifer Messerly, 2015/06/05 22:45:57]

couple of thoughts here:

Maybe pass "null" instead of "global"? I don't think we generate module-level
`this`, so shouldn't affect anything?

It looks like we're using eager loaded of the library? Seems fine. It's
interesting though, that we could load lazily if we wanted, because we have
enough info to do so, and trigger it all from "start" or "import". Not sure if
that's worth a comment of some sort.

[vsm, 2015/06/05 23:03:11]

Done.

I'll tackle ordering (lazy or eager) in a separate CL.

+  }
+  dart.library = library;
+
+  function start(libraryName) {
+    let lib = import_(libraryName);
+    _isolate_helper.startRootIsolate(lib.main, []);
   }
-  dart.defineLibrary = defineLibrary;
+  dart.start = start;
 
   // TODO(jmesserly): hack to bootstrap the SDK

[Jennifer Messerly, 2015/06/05 22:45:57]

is this still needed?

[vsm, 2015/06/05 23:03:10]

Done.

-  _js_helper = _js_helper || {};
+  let core = lazyImport('dart/core.js');
+  let collection = lazyImport('dart/collection.js');
+  let async = lazyImport('dart/async.js');
+  let _interceptors = lazyImport('dart/_interceptors.js');
+  let _isolate_helper = lazyImport('dart/_isolate_helper.js');
+  let _js_helper = lazyImport('dart/_js_helper.js');
   _js_helper.checkNum = notNull;
-
-  _js_primitives = _js_primitives || {};
+  let _js_primitives = lazyImport('dart/_js_primitives.js');
   _js_primitives.printString = (s) => console.log(s);
 
   // TODO(vsm): DOM facades?