frog isolate fixes: minor changes to the isolate library + architecture.py

frog/world.dart

 // Copyright (c) 2011, 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.
 
 /** The one true [World]. */
 World world;
 
 /**
  * Experimental phase to enable await, only set when using the
  * await/awaitc.dart entrypoint.
@@ skipping 197 matching lines @@
       // resolve conflicts based on priority
       int existingPri = existing.jsnamePriority;
       int namedPri = named.jsnamePriority;
       if (existingPri > namedPri || namedPri == 0) {
         // Either existing was higher priority, or they're both 0 so first one
         // wins.
         _renameJavascriptTopName(named);
       } else if (namedPri > existingPri) {
         // New one takes priority over existing
         _renameJavascriptTopName(existing);
-      } else {
+      } else if ((!_isDomWorker(named) || !_isIsolateWorker(existing))
+          && (!_isDomWorker(existing) || !_isIsolateWorker(named))) {

[mattsh, 2011/12/22 01:55:15]

is there a way to simplify this?  (Read a few times and can't quite follow it.)

[Siggi Cherem (dart-lang), 2011/12/22 18:51:15]

How about this (not-normalized form is a bit easier to read):

if (!((_isDomWorker(named) && _isIsolateWorker(existing)
|| (_isDomWorker(existing) && _isIsolateWorker(named))) {
 ...
}

[Jennifer Messerly, 2011/12/22 19:49:44]

How about:

} else if (!named.isWebWorker && !existing.isWebWorker) {
...
}

we shouldn't ever get in the state where DOM Worker is conflicting with DOM
Worker ... so that collapses the two cases

         // Two conflicting native names or names in corelib. Libraries need
-        // to be fixed.
+        // to be fixed. We exclude the case when isolate defines Worker, so that
+        // corelib doesn't depend on the dom lib.
         world.internalError('conflicting JS name "$name" of same '
             + 'priority $existingPri: (already defined in) '
             + '${existing.span.locationText} with priority $namedPri)',
             named.span, existing.span);
       }
     } else {
       // No one was using the name. Take it for ourselves.
       _topNames[name] = named;
     }
   }
 
+  /** Whether element is the dom definition of Worker. */
+  _isDomWorker(Element e) {

[Jennifer Messerly, 2011/12/22 19:49:44]

personally I'd put this as a getter on Element. Even though we only use it from
world right now: "named.isWebWorker" reads better imho (or "isWorker" -- but
Worker is kind of generic, and most people are familiar with the "web worker"
term)

[Siggi Cherem (dart-lang), 2011/12/22 21:17:23]

Seems really strange to add this on Element. I don't know if this will be the
only type conflict that we will ever have though. An alternative I was
discussing with Matt was to change the if test above to be:

if (!_isConflictHarmless(named, element)) {
  ...
}

and define here:
_isConflictHarmless(e1, e2) {
  if (e1.library != dom && e1.library != corelib) return false;
  if (e2.library != dom && e2.library != corelib) return false;
  return e1.nativeName == 'Worker' && e2.nativeName == 'Worker';
}

+    return e.library == dom && e.nativeName == 'Worker';
+  }
+
+  /** Whether element is the isolate (corelib) definition of Worker. */
+  _isIsolateWorker(Element e) {
+    return e.library == coreimpl && e.nativeName == 'Worker';
+  }
+
   /** Renames an [Element] that had a name conflict in the generated JS. */
   _renameJavascriptTopName(Element named) {
     named._jsname = '${named.library.jsname}_${named.jsname}';
     final existing = _topNames[named.jsname];
     if (existing != null && existing != named) {
       // If this happens it means the library name wasn't unique enough.
       world.internalError('name mangling failed for "${named.jsname}" '
           + '("${named.jsname}" defined also in ${existing.span.locationText})',
           named.span);
     }
@@ skipping 302 matching lines @@
 
   withTiming(String name, f()) {
     final sw = new Stopwatch();
     sw.start();
     var result = f();
     sw.stop();
     info('$name in ${sw.elapsedInMs()}msec');
     return result;
   }
 }