dart2js cps: 'is' checks on types with type arguments.

pkg/compiler/lib/src/js_backend/codegen/codegen.dart

Index: pkg/compiler/lib/src/js_backend/codegen/codegen.dart
diff --git a/pkg/compiler/lib/src/js_backend/codegen/codegen.dart b/pkg/compiler/lib/src/js_backend/codegen/codegen.dart
index 0a2242477998b51f0e5c8b0db4dc789a28998829..df2fa7b9e9bd618fe64dd61d63915ba5205f9e2a 100644
--- a/pkg/compiler/lib/src/js_backend/codegen/codegen.dart
+++ b/pkg/compiler/lib/src/js_backend/codegen/codegen.dart
@@ -350,14 +350,44 @@ class CodeGenerator extends tree_ir.StatementVisitor
 
   @override
   js.Expression visitTypeOperator(tree_ir.TypeOperator node) {
+    js.Expression value = visitExpression(node.value);
+    List<js.Expression> typeArguments =
+        node.typeArguments.map(visitExpression).toList();
     if (!node.isTypeTest) {
       giveup(node, 'type casts not implemented.');
     }
     DartType type = node.type;
-    if (type is InterfaceType && type.isRaw) {
+    // Note that the trivial (but special) cases of Object, dynamic, and Null
+    // are handled at build-time and must not occur in a TypeOperator.
+    assert(!type.isObject && !type.isDynamic);

[karlklose, 2015/05/27 07:38:34]

Also assert(type is! InterfaceType || (type as InterfaceType).element !=
nullClass)?

[asgerf, 2015/05/27 09:19:16]

The assertions are mostly there to add clarity. I left this one out because I
felt such a verbose assertion would have the opposite effect.

We would also get a method in the glue class that we don't actually need
(getting the null class), which kind of defeats the purpose of having the glue
class.

+    if (type is InterfaceType) {

[karlklose, 2015/05/27 07:38:35]

I think we should keep the case for no type arguments separate and always
provide all arguments to the helper.

Using less arguments than declared is not free in JavaScript engines and we can
add a different helper if the explicit null literals turn out to be a problem
with code size.  Also, we do not need to emit the helper, if there are no
generic type checks in the program.

[asgerf, 2015/05/27 09:19:16]

I should have clarified in the CL description.

The case for a non-generic type will be detected in a CPS optimization phase and
replaced by a specialized IR node. The reason being that the value should
usually be intercepted (and hence shared if possible) in the non-generic case,
but *only* in the non-generic case. In the generic case, the interceptor is not
needed.

And we don't want the TypeOperator to have some silly condition like "the value
must be an interceptor unless there are type arguments".

Good point about the missing arguments. I'll insert nulls.

       glue.registerIsCheck(type, registry);
-      js.Expression value = visitExpression(node.receiver);
-      return js.js('!!#.#', [value, glue.getTypeTestTag(type)]);
+      ClassElement clazz = type.element;
+
+      // We use the helper:
+      //
+      //     checkSubtype(value, $isT, typeArgs, $asT)
+      //
+      // Any of the last two arguments may be omitted if there are no type
+      // arguments, and/or if no substitution is required.
+
+      js.Expression isT = js.string(glue.getTypeTestTag(type));
+      List<js.Expression> checkSubtypeArgs = [value, isT];
+
+      // If the type has type arguments, check the type arguments.
+      if (typeArguments.isNotEmpty) {
+        checkSubtypeArgs.add(new js.ArrayInitializer(typeArguments));
+
+        // If the type has any subclasses, the type arguments need to be
+        // computed using the $asT method.
+        if (glue.hasStrictSubtype(clazz)) {
+          js.Expression asT = js.string(glue.getTypeSubstitutionTag(clazz));
+          checkSubtypeArgs.add(asT);
+        }
+      }
+      return buildStaticHelperInvocation(
+          glue.getCheckSubtype(),
+          checkSubtypeArgs);
     }
     return giveup(node, 'type check unimplemented for $type.');
   }