improve DDC's type checks

pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/rtti.dart

Index: pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/rtti.dart
diff --git a/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/rtti.dart b/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/rtti.dart
index 521716bdf06a9820bdcfe7f0d26c0d3220f61c94..616f5251f59566d8071d5dc3e5272bb9545dcf07 100644
--- a/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/rtti.dart
+++ b/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/rtti.dart
@@ -75,39 +75,6 @@ final _runtimeType = JS('', 'Symbol("_runtimeType")');
 
 final _moduleName = JS('', 'Symbol("_moduleName")');
 
-_checkPrimitiveType(obj) {
-  // TODO(jmesserly): JS is used to prevent type literal wrapping.  Is there a
-  // better way we can handle this?  (sra: It is super dodgy that the values
-  // passed to JS are different to the values passed to a regular function - the
-  // semantics are not longer that of calling an interpreter. dart2js has other
-  // special functions, we could do the same.)
-
-  // Check for null and undefined
-  if (obj == null) return JS('', '#', Null);
-
-  if (JS('bool', 'typeof # == "number"', obj)) {
-    if (JS('bool', 'Math.floor(#) == #', obj, obj)) {
-      return JS('', '#', int);
-    }
-    return JS('', '#', double);
-  }
-
-  if (JS('bool', 'typeof # == "boolean"', obj)) {
-    return JS('', '#', bool);
-  }
-
-  if (JS('bool', 'typeof # == "string"', obj)) {
-    return JS('', '#', String);
-  }
-
-  if (JS('bool', 'typeof # == "symbol"', obj)) {
-    // Note: this is a JS Symbol, not a Dart one.
-    return JS('', '#', jsobject);
-  }
-
-  return null;
-}
-
 getFunctionType(obj) {
   // TODO(vsm): Encode this properly on the function for Dart-generated code.
   var args = JS('List', 'Array(#.length).fill(#)', obj, dynamic);
@@ -120,34 +87,32 @@ getFunctionType(obj) {
 /// different from the user-visible Type object returned by calling
 /// `runtimeType` on some Dart object.
 getReifiedType(obj) {
-  var result = _checkPrimitiveType(obj);
-  if (result != null) return result;
-  return _nonPrimitiveRuntimeType(obj);
-}
-
-/// Assumes that obj is non-null
-_nonPrimitiveRuntimeType(obj) {
-  // Lookup recorded *real* type (not user definable runtimeType)
-  // TODO(vsm): Should we treat Dart and JS objects differently here?
-  // E.g., we can check if obj instanceof core.Object to differentiate.
-  var result = _getRuntimeType(obj);
-  if (result != null) return result;
-
-  // Lookup extension type
-  result = getExtensionType(obj);
-  if (result != null) return result;
-
-  // Fallback on constructor for class types
-  result = JS('', '#.constructor', obj);
-  if (JS('bool', '# === Function', result)) {
-    // An undecorated Function should have come from JavaScript.
-    // Treat as untyped.
-    return JS('', '#', jsobject);
-  }
-  if (result == null) {
-    return JS('', '#', jsobject);
+  switch (JS('String', 'typeof #', obj)) {

[Leaf, 2017/08/24 17:19:39]

Will V8 make this code fast, even when the `typeof` is separated from the
equality check on the outcome?  That is, is this as good as (or better than)
checking `if (typeof obj === string)` in each of the branches for different
values of `string`?  I assume so, but just want to be sure, since this is
probably pretty critical code.

[Jennifer Messerly, 2017/08/24 18:26:37]

Yup. I was skeptical too, but this code was the result of a lot of testing this
with a microbench that stressed getReifiedType.

+    case "object":
+      if (obj == null) return JS('', '#', Null);
+      if (JS('bool', '# instanceof #', obj, Object)) {
+        return JS('', '#.constructor', obj);
+      }
+      var result = JS('', '#[#]', obj, _extensionType);
+      if (result == null) return JS('', '#', jsobject);

[Jennifer Messerly, 2017/08/23 23:36:01]

FYI -- there's one subtle difference from the old code.

In the old code, we allowed JS constructors from types we don't know anything
about to escape into our type system. These objects didn't really work as types
for us -- they don't have is/as/_check, for example. Our type functions have
various scattered workarounds. Now, we check for`obj instanceof core.Object`,
which finds Dart objects.

This is basically fixing vsm's TODO in the old code. :)

This does mean all JS constructors now have `jsobject` as their RTTI, unless
they have a corresponding _extensionType pointer.

In practice this works out, because we use LazyJSType or AnonymousJSType to
represent these in the type system. Both of those types understand how to check
JS objects.

If we did want to reify something more precise, though, we could do it here.
Perhaps we could record the `obj.constructor` property in the JSObject type.

Ultimately my goal here was to make normal Dart objects fast, as that's 99% of
what we're dealing with. And I want JS objects to return a TypeRep, rather than
letting JavaScript constructors float around in our type system. One can imagine
a future where we want that level of interop, but I don't think we're there, and
having a clear boundary between Dart and JS types will make a lot of things more
sane in our RTTI code.

[Leaf, 2017/08/24 17:19:39]

Acknowledged.

+      return result;
+    case "function":
+      // All Dart functions and callable classes must set _runtimeType
+      var result = JS('', '#[#]', obj, _runtimeType);
+      if (result != null) return result;
+      return JS('', '#', jsobject);
+    case "undefined":
+      return JS('', '#', Null);
+    case "number":
+      return JS('', 'Math.floor(#) == # ? # : #', obj, obj, int, double);
+    case "boolean":
+      return JS('', '#', bool);
+    case "string":
+      return JS('', '#', String);
+    case "symbol":
+    default:
+      return JS('', '#', jsobject);
   }
-  return result;
 }
 
 /// Given an internal runtime type object, wraps it in a `WrappedType` object