Fix algorithm to know whether we should emit an is check on a class: all interfaces/implemented cla…

lib/compiler/implementation/js_backend/emitter.dart

Index: lib/compiler/implementation/js_backend/emitter.dart
===================================================================
--- lib/compiler/implementation/js_backend/emitter.dart	(revision 12536)
+++ lib/compiler/implementation/js_backend/emitter.dart	(working copy)
@@ -701,29 +701,34 @@
     if (checkedClasses.contains(cls)) {
       generateTypeTest(cls);
     }
-    generateInterfacesIsTests(cls, generateTypeTest, new Set<Element>());
+    Set<Element> generated = new Set<Element>();
+    for (DartType interfaceType in cls.interfaces) {
+      generateInterfacesIsTests(

[Lasse Reichstein Nielsen, 2012/09/19 09:52:10]

Why is it called "IsTests" in the called function's name, but "TypeTests" in
this function's name? Is it the same thing?

[ngeoffray, 2012/09/19 10:14:39]

Renamed everything to IsTest instead of TypeTest.

+          interfaceType.element, generateTypeTest, generated);
+    }
   }
 
   void generateInterfacesIsTests(ClassElement cls,

[Lasse Reichstein Nielsen, 2012/09/19 09:52:10]

Please document what the function does.
And the function above too.
I can't see if what is does is correct without knowing what it should do, and
the name is not sufficient here.

[ngeoffray, 2012/09/19 10:14:39]

Done.

                                  void generateTypeTest(ClassElement element),
                                  Set<Element> alreadyGenerated) {
+    void tryEmitTest(ClassElement cls) {
+      if (!alreadyGenerated.contains(cls) && checkedClasses.contains(cls)) {
+        alreadyGenerated.add(cls);
+        generateTypeTest(cls);
+      }
+    };
+
     for (DartType interfaceType in cls.interfaces) {
       Element element = interfaceType.element;
-      if (!alreadyGenerated.contains(element) &&
-          checkedClasses.contains(element)) {
-        alreadyGenerated.add(element);
-        generateTypeTest(element);
-      }
+      tryEmitTest(element);
       generateInterfacesIsTests(element, generateTypeTest, alreadyGenerated);
+    }
 
-      // Since [element] is implemented by [cls], we need to also emit
-      // is checks for the superclass and its supertypes.
-      ClassElement superclass = element.superclass;
-      if (!alreadyGenerated.contains(superclass) &&
-          checkedClasses.contains(superclass)) {
-        alreadyGenerated.add(superclass);
-        generateTypeTest(superclass);
-      }
+    // We need to also emit

[Lasse Reichstein Nielsen, 2012/09/19 09:52:10]

Fix linebreaking. Put quotes around "is-checks" or something similar to make it
stand out from the normal words.

[ngeoffray, 2012/09/19 10:14:39]

Done.

+    // is checks for the superclass and its supertypes.
+    ClassElement superclass = cls.superclass;
+    if (superclass != null) {
+      tryEmitTest(superclass);
       generateInterfacesIsTests(superclass, generateTypeTest, alreadyGenerated);
     }
   }