Track emitted classes in incremental compiler.

dart/pkg/dart2js_incremental/lib/library_updater.dart

 // Copyright (c) 2014, 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.
 
 library dart2js_incremental.library_updater;
 
 import 'dart:async' show
     Future;
 
 import 'dart:convert' show
@@ skipping 107 matching lines @@
 
   final List<FailedUpdate> _failedUpdates = <FailedUpdate>[];
 
   final Set<ElementX> _elementsToInvalidate = new Set<ElementX>();
 
   final Set<ElementX> _removedElements = new Set<ElementX>();
 
   final Set<ClassElementX> _classesWithSchemaChanges =
       new Set<ClassElementX>();
 
-  final Set<ClassElementX> _existingClasses = new Set();
+  final Set<ClassElementX> _emittedClasses;
+
+  final Set<ClassElementX> _directlyInstantiatedClasses;
 
   LibraryUpdater(
-      this.compiler,
+      Compiler compiler,
       this.inputProvider,
       this.uri,
       this.logTime,
-      this.logVerbose) {
-    _existingClasses.addAll(emitter.neededClasses);
-  }
+      this.logVerbose)
+      : this.compiler = compiler,
+        _emittedClasses = new Set.from(compiler.backend.emitter.neededClasses),
+        _directlyInstantiatedClasses = new Set.from(
+            compiler.codegenWorld.directlyInstantiatedClasses);
 
   /// When [true], updates must be applied (using [applyUpdates]) before the
   /// [compiler]'s state correctly reflects the updated program.
   bool get hasPendingUpdates => !updates.isEmpty;
 
   bool get failed => !_failedUpdates.isEmpty;
 
   /// Used as tear-off passed to [LibraryLoaderTask.resetAsync].
   Future<bool> reuseLibrary(LibraryElement library) {
     assert(compiler != null);
@@ skipping 411 matching lines @@
       if (!element.isClass) {
         compiler.enqueuer.codegen.addToWorkList(element);
       } else {
         changedClasses.add(element);
       }
     }
     compiler.processQueue(compiler.enqueuer.codegen, null);
 
     for (Element e in compiler.enqueuer.codegen.generatedCode.keys) {
       if (e.isFunction && e.functionSignature.hasOptionalParameters) {
         for (Selector selector in compiler.enqueuer.codegen.newlySeenSelectors) {

[floitsch, 2014/12/01 13:12:29]

long line.

[ahe, 2014/12/01 13:58:52]

Done.

           // TODO(ahe): Group selectors by name at this point for improved
           // performance.
           if (e.name == selector.name &&
               selector.appliesUnnamed(e, compiler.world)) {
             // TODO(ahe): Don't use
             // compiler.enqueuer.codegen.newlyEnqueuedElements directly like
             // this, make a copy.
             compiler.enqueuer.codegen.newlyEnqueuedElements.add(e);
           }
           if (selector.name == namer.closureInvocationSelectorName) {
             selector = new Selector.call(
                 e.name, e.library,
                 selector.argumentCount, selector.namedArguments);
             if (selector.appliesUnnamed(e, compiler.world)) {
               // TODO(ahe): Also make a copy here.
               compiler.enqueuer.codegen.newlyEnqueuedElements.add(e);
             }
           }
         }
       }
     }
 
     List<jsAst.Statement> updates = <jsAst.Statement>[];
 
-    // TODO(ahe): allInstantiatedClasses seem to include interfaces that aren't
-    // needed.
     Set<ClassElementX> newClasses = new Set.from(
-        compiler.codegenWorld.allInstantiatedClasses.where(
-            emitter.computeClassFilter()));
-    newClasses.removeAll(_existingClasses);
+        compiler.codegenWorld.directlyInstantiatedClasses);
+    newClasses.removeAll(_directlyInstantiatedClasses);
 
-    // TODO(ahe): When more than one updated is computed, we need to make sure
-    // that existing classes aren't overwritten. No test except the one test
-    // that tests more than one update is affected by this problem, and only
-    // because main is closurized because we always enable tear-off. Is that
-    // really necessary? Also, add a test which tests directly that what
-    // happens when tear-off is introduced in second update.
-    emitter.neededClasses.addAll(newClasses);
+    if (!newClasses.isEmpty) {
+      // Ask the emitter to compute "needs" (only) if new classes were

[karlklose, 2014/12/01 10:13:32]

Please document what "needs" are.

[ahe, 2014/12/01 10:28:35]

I'm being vague because I don't know precisely.

+      // instantiated.
+      emitter.computeNeeds();
+      newClasses = new Set.from(emitter.neededClasses);
+      newClasses.removeAll(_emittedClasses);
+    } else {
+      // Make sure that the set of emitted classes is preserved for subsequent
+      // updates.
+      // TODO(ahe): This is a bit convoluted, find a better approach.
+      emitter.neededClasses

[karlklose, 2014/12/01 10:13:31]

Why not `new Set.fom`?

[ahe, 2014/12/01 10:28:35]

The field is final.

+          ..clear()
+          ..addAll(_emittedClasses);
+    }
 
     List<jsAst.Statement> inherits = <jsAst.Statement>[];
 
     for (ClassElementX cls in newClasses) {
       jsAst.Node classAccess = namer.elementAccess(cls);
       String name = namer.getNameOfClass(cls);
 
       updates.add(
           js.statement(
               r'# = #', [classAccess, invokeDefineClass(cls)]));
@@ skipping 614 matching lines @@
   List<String> computeFields(ClassElement cls) {
     // TODO(ahe): Rewrite for new emitter.
     ClassBuilder builder = new ClassBuilder(cls, namer);
     classEmitter.emitFields(cls, builder, "");
     return builder.fields;
   }
 }
 
 // TODO(ahe): Remove this method.
 NO_WARN(x) => x;