Delete instance methods.

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 69 matching lines @@
   FailedUpdate(this.context, this.message);
 
   String toString() {
     if (context == null) return '$message';
     return 'In $context:\n  $message';
   }
 }
 
 // TODO(ahe): Generalize this class. For now only works for Compiler.mainApp,
 // and only if that library has exactly one compilation unit.
-class LibraryUpdater {
+class LibraryUpdater extends JsFeatures {
   final Compiler compiler;
 
   final api.CompilerInputProvider inputProvider;
 
   final Logger logTime;
 
   final Logger logVerbose;
 
   // TODO(ahe): Get rid of this field. It assumes that only one library has
   // changed.
   final Uri uri;
 
   final List<Update> updates = <Update>[];
 
   final List<FailedUpdate> _failedUpdates = <FailedUpdate>[];
 
   final Set<ElementX> _elementsToInvalidate = new Set<ElementX>();
 
+  final Set<ElementX> _removedElements = new Set<ElementX>();
+
   LibraryUpdater(
       this.compiler,
       this.inputProvider,
       this.uri,
       this.logTime,
       this.logVerbose);
 
   /// 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;
 
-  JavaScriptBackend get backend => compiler.backend;
-
-  Namer get namer => backend.namer;
-
-  CodeEmitterTask get emitter => backend.emitter;
-
   /// Used as tear-off passed to [LibraryLoaderTask.resetAsync].
   Future<bool> reuseLibrary(LibraryElement library) {
     assert(compiler != null);
     if (library.isPlatformLibrary || library.isPackageLibrary) {
       logTime('Reusing $library.');
       return new Future.value(true);
     } else if (library != compiler.mainApp) {
       return new Future.value(false);
     }
     return inputProvider(uri).then((bytes) {
@@ skipping 105 matching lines @@
     for (ScopeContainerElement scope in scopesAffectedBy(element, cls)) {
       scanSites(scope, (Element member, DeclarationSite site) {
         // TODO(ahe): Cache qualifiedNamesIn to avoid quadratic behavior.
         Map<String, List<String>> names = qualifiedNamesIn(site);
         if (canNamesResolveTo(names, element, cls)) {
           _elementsToInvalidate.add(member);
         }
       });
     }
 
-    // TODO(ahe): Don't modify the class here, instead use an instance of
-    // Update.
-    Link<Element> localMembersReversed = const Link<Element>();
-    cls.forEachLocalMember((member) {
-      if (member != element) {
-        localMembersReversed = localMembersReversed.prepend(member);
-      }
-    });
-    cls.localMembersCache = null;
-    cls.localMembersReversed = localMembersReversed;
-    cls.localScope.contents.remove(element.name);
+    _removedElements.add(element);
 
-    // TODO(ahe): Also compute a patch which removes the function, e.g.,
-    // "delete GlobalObject.MyClass.prototype.memberName".
-
-    // TODO(ahe): Also forget [element].
+    updates.add(new RemovedFunctionUpdate(compiler, element));
 
     return true;
   }
 
   void scanSites(
       Element element,
       void f(ElementX element, DeclarationSite site)) {
     DeclarationSite site = declarationSite(element);
     if (site != null) {
       f(element, site);
@@ skipping 77 matching lines @@
   bool unableToReuse(
       Token diffToken,
       PartialElement before,
       PartialElement after) {
     return cannotReuse(
         after,
         'Unhandled change:'
         ' ${before} (${before.runtimeType} -> ${after.runtimeType}).');
   }
 
-  List<Element> applyUpdates() {
+  List<Element> applyUpdates([List<Update> removals]) {

[Johnni Winther, 2014/11/14 08:55:26]

Document this, especially semantics of the return value.

[ahe, 2014/11/14 14:33:12]

Will do in a follow-up.

+    for (Update update in updates) {
+      update.captureState();
+    }
     if (!_failedUpdates.isEmpty) {
       throw new StateError(
           "Can't compute update.\n\n${_failedUpdates.join('\n\n')}");
     }
     for (ElementX element in _elementsToInvalidate) {
       compiler.forgetElement(element);
       element.reuseElement();
     }
-    return updates.map((Update update) => update.apply()).toList()
-        ..addAll(_elementsToInvalidate);
+    List<Element> elementsToInvalidate = <Element>[];
+    for (ElementX element in _elementsToInvalidate) {
+      if (!_removedElements.contains(element)) {
+        elementsToInvalidate.add(element);
+      }
+    }
+    for (Update update in updates) {
+      Element element = update.apply();
+      if (update.isRemoval) {
+        if (removals != null) {
+          removals.add(update);
+        }
+      } else {
+        elementsToInvalidate.add(element);
+      }
+    }
+    return elementsToInvalidate;
   }
 
   String computeUpdateJs() {
-    List<Element> updatedElements = applyUpdates();
+    List<Update> removals = <Update>[];
+    List<Element> updatedElements = applyUpdates(removals);
     if (compiler.progress != null) {
       compiler.progress.reset();
     }
     for (Element element in updatedElements) {
       compiler.enqueuer.resolution.addToWorkList(element);
     }
     compiler.processQueue(compiler.enqueuer.resolution, null);
 
     compiler.phase = Compiler.PHASE_DONE_RESOLVING;
 
     for (Element element in updatedElements) {
       compiler.enqueuer.codegen.addToWorkList(element);
     }
     compiler.processQueue(compiler.enqueuer.codegen, null);
 
     List<jsAst.Statement> updates = <jsAst.Statement>[];
     for (Element element in compiler.enqueuer.codegen.newlyEnqueuedElements) {
       if (!element.isField) {
         updates.add(computeMemberUpdateJs(element));
       }
     }
+    for (RemovedFunctionUpdate update in removals) {
+      update.writeUpdateJsOn(updates);
+    }
 
     if (updates.length == 1) {
       return prettyPrintJs(updates.single);
     } else {
       return prettyPrintJs(js.statement('{#}', [updates]));
     }
   }
 
   jsAst.Node computeMemberUpdateJs(Element element) {
     MemberInfo info = emitter.oldEmitter.containerBuilder
@@ skipping 57 matching lines @@
   final Compiler compiler;
 
   PartialElement get before;
 
   PartialElement get after;
 
   Update(this.compiler);
 
   /// Applies the update to [before] and returns that element.
   PartialElement apply();
+
+  bool get isRemoval => false;
+
+  /// Called before any patches are applied to capture any state that is needed
+  /// later.
+  void captureState() {
+  }
 }
 
 /// Represents an update of a function element.
-class FunctionUpdate extends Update {
+class FunctionUpdate extends Update with ReuseFunction {
   final PartialFunctionElement before;
 
   final PartialFunctionElement after;
 
   FunctionUpdate(Compiler compiler, this.before, this.after)
       : super(compiler);
 
   PartialFunctionElement apply() {
     patchElement();
     reuseElement();
     return before;
   }
 
   /// Destructively change the tokens in [before] to match those of [after].
   void patchElement() {
     before.beginToken = after.beginToken;
     before.endToken = after.endToken;
     before.getOrSet = after.getOrSet;
   }
+}
+
+abstract class ReuseFunction {
+  PartialFunctionElement get before;
 
   /// Reset various caches and remove this element from the compiler's internal
   /// state.
   void reuseElement() {
     compiler.forgetElement(before);
     before.reuseElement();
   }
 }
 
+class RemovedFunctionUpdate extends Update with JsFeatures, ReuseFunction {
+  final PartialFunctionElement element;
+
+  /// Name of property to remove using JavaScript "delete".
+  String name;
+
+  /// Name of super-alias property to remove using JavaScript "delete".  Null
+  /// for methods that aren't "super aliased" (should imply that this field is
+  /// null for all non-instance methods).
+  String superName;
+
+  bool wasStateCaptured = false;
+
+  RemovedFunctionUpdate(Compiler compiler, this.element)
+      : super(compiler);
+
+  PartialFunctionElement get before => element;
+
+  PartialElement get after => null;
+
+  bool get isRemoval => true;
+
+  void captureState() {
+    if (wasStateCaptured) throw "captureState was called twice.";
+
+    if (element.isInstanceMember) {
+      name = namer.getNameOfMember(element);
+    }
+    if (backend.isAliasedSuperMember(element)) {
+      superName = namer.getNameOfAliasedSuperMember(element);
+    }
+
+    wasStateCaptured = true;
+  }
+
+  PartialElement apply() {
+    if (!wasStateCaptured) throw "captureState must be called before apply.";
+    removeFromEnclosingClass();
+    reuseElement();
+    return null;
+  }
+
+  void removeFromEnclosingClass() {
+    PartialClassElement cls = element.enclosingClass;
+
+    Link<Element> localMembersReversed = const Link<Element>();
+    bool foundElement = false;
+    cls.forEachLocalMember((member) {
+      if (member != element) {
+        localMembersReversed = localMembersReversed.prepend(member);
+      } else {
+        if (foundElement) {
+          throw "Found '$element' twice in '$cls'.";
+        }
+        foundElement = true;
+      }
+    });
+    if (!foundElement) {
+      throw "Don't find '$element' in '$cls'.";
+    }
+    cls.localMembersCache = null;
+    cls.localMembersReversed = localMembersReversed;
+    cls.localScope.contents.remove(element.name);
+
+    return null;
+  }
+
+  void writeUpdateJsOn(List<jsAst.Statement> updates) {
+    if (name == null) {
+      compiler.internalError(element, '${element.runtimeType}');
+    }
+    if (element.isInstanceMember) {
+      jsAst.Node elementAccess = namer.elementAccess(element.enclosingClass);
+      updates.add(
+          js.statement('delete #.prototype.#', [elementAccess, name]));
+
+      if (superName != null) {
+        updates.add(
+            js.statement('delete #.prototype.#', [elementAccess, superName]));
+      }
+    } else {
+      compiler.internalError(
+          element, 'Removal of non-instance methods not yest supported.');

[Johnni Winther, 2014/11/14 08:55:26]

non-instance -> static

[ahe, 2014/11/14 14:33:12]

You missed the misspelling of "yet" ;-)

Regardless, this goes away in one of the other CL you have already reviewed, or
will review soon.

But there's a reason why I use "non-instance" instead of "static". In our world,
we consider a top-level method to be a static method, yet the keyword "static"
isn't legal on a top-level method. So when formatting an error message, I prefer
saying "static or top-level", but I figured "non-instance" would be just as
descriptive. But I'm not sure that's actually true.

+    }
+  }
+}
+
 Map<String, List<String>> qualifiedNamesIn(PartialElement element) {
   Token beginToken = element.beginToken;
   Token endToken = element.endToken;
   Token token = beginToken;
   if (element is PartialClassElement) {
     ClassNode node = element.cachedNode;
     if (node != null) {
       NodeList body = node.body;
       if (body != null) {
         endToken = body.beginToken;
@@ skipping 30 matching lines @@
       // of [element].
       return true;
     }
   }
   return false;
 }
 
 DeclarationSite declarationSite(Element element) {
   return element is ElementX ? element.declarationSite : null;
 }
+
+abstract class JsFeatures {
+  Compiler get compiler;
+
+  JavaScriptBackend get backend => compiler.backend;
+
+  Namer get namer => backend.namer;
+
+  CodeEmitterTask get emitter => backend.emitter;
+}