Patch refactoring.

lib/compiler/implementation/elements/elements.dart

Index: lib/compiler/implementation/elements/elements.dart
diff --git a/lib/compiler/implementation/elements/elements.dart b/lib/compiler/implementation/elements/elements.dart
index 4bed08dbcd52e3cd2dae94dc80666b8befea235a..03b3e2854284d14763dccc33872ab95dc0a2127f 100644
--- a/lib/compiler/implementation/elements/elements.dart
+++ b/lib/compiler/implementation/elements/elements.dart
@@ -114,13 +114,14 @@ class Element implements Hashable, Spannable {
   Link<MetadataAnnotation> metadata = const EmptyLink<MetadataAnnotation>();
 
   Element(this.name, this.kind, this.enclosingElement) {
-    assert(getLibrary() !== null);
+    assert(isErroneous() || getImplementationLibrary() !== null);
   }
 
   Modifiers get modifiers => null;
 
   Node parseNode(DiagnosticListener listener) {
-    listener.cancel("Internal Error: $this.parseNode", token: position());
+    listener.cancel("Internal Error: $this.parseNode not "
+                    "implemented on ${super.toString()}", token: position());
   }
 
   DartType computeType(Compiler compiler) {
@@ -189,21 +190,20 @@ class Element implements Hashable, Spannable {
    */
   bool get isPatch => false;
 
-
   /**
    * Is [:true:] if this element defines the implementation for the entity of
    * this element.
    *
    * See [:patch_parser.dart:] for a description of the terminology.
    */
-  bool get isImplementation => implementation === this;
+  bool get isImplementation => !isPatched;
 
   /**
    * Is [:true:] if this element introduces the entity of this element.
    *
    * See [:patch_parser.dart:] for a description of the terminology.
    */
-  bool get isDeclaration => declaration === this;
+  bool get isDeclaration => !isPatch;
 
   /**
    * Returns the element which defines the implementation for the entity of this
@@ -211,14 +211,14 @@ class Element implements Hashable, Spannable {
    *
    * See [:patch_parser.dart:] for a description of the terminology.
    */
-  Element get implementation => this;
+  Element get implementation => isPatched ? patch : this;
 
   /**
    * Returns the element which introduces the entity of this element.
    *
    * See [:patch_parser.dart:] for a description of the terminology.
    */
-  Element get declaration => this;
+  Element get declaration => isPatch ? origin : this;
 
   // TODO(johnniwinther): This breaks for libraries (for which enclosing
   // elements are null) and is invalid for top level variable declarations for
@@ -260,17 +260,13 @@ class Element implements Hashable, Spannable {
         return library.entryCompilationUnit;
       }
       element = element.enclosingElement;
-      if (element is FunctionElement) {
-        FunctionElement function = element;
-        if (function.isPatched) {
-          element = function.patch;
-        }
-      }
     }
     return element;
   }
 
-  LibraryElement getLibrary() {
+  LibraryElement getLibrary() => enclosingElement.getLibrary();
+
+  LibraryElement getImplementationLibrary() {
     Element element = this;
     while (element.kind !== ElementKind.LIBRARY) {
       element = element.enclosingElement;
@@ -278,8 +274,6 @@ class Element implements Hashable, Spannable {
     return element;
   }
 
-  LibraryElement getImplementationLibrary() => getLibrary();
-
   ClassElement getEnclosingClass() {
     for (Element e = this; e !== null; e = e.enclosingElement) {
       if (e.isClass()) return e;
@@ -315,12 +309,14 @@ class Element implements Hashable, Spannable {
    * Creates the scope for this element. The scope of the
    * enclosing element will be the parent scope.
    */
-  Scope buildScope() => buildEnclosingScope();
+  Scope buildScope([bool patchScope = false]) =>

[ahe, 2012/09/24 12:30:18]

I don't like this optional argument. I think you want to clean it up later, so
how about adding a TODO?

[ahe, 2012/09/24 12:30:18]

Use {}

[Johnni Winther, 2012/09/25 09:01:07]

Done.

[Johnni Winther, 2012/09/25 09:01:07]

Done.

+      buildEnclosingScope(patchScope);
 
   /**
    * Creates the scope for the enclosing element.
    */
-  Scope buildEnclosingScope() => enclosingElement.buildScope();
+  Scope buildEnclosingScope([bool patchScope = false]) =>

[ahe, 2012/09/24 12:30:18]

Ditto.

[ahe, 2012/09/24 12:30:18]

Use {}

[Johnni Winther, 2012/09/25 09:01:07]

Done.

[Johnni Winther, 2012/09/25 09:01:07]

Done.

+      enclosingElement.buildScope(patchScope);
 
   String toString() {
     // TODO(johnniwinther): Test for nullness of name, or make non-nullness an
@@ -342,6 +338,7 @@ class Element implements Hashable, Spannable {
 
   FunctionElement asFunctionElement() => null;
 
+  static bool isInvalid(Element e) => e == null || e.isErroneous();
   Element cloneTo(Element enclosing, DiagnosticListener listener) {
     listener.cancel("Unimplemented cloneTo", element: this);
   }
@@ -444,7 +441,11 @@ class ScopeContainerElement extends ContainerElement {
   }
 
   Element localLookup(SourceString elementName) {
-    return localScope[elementName];
+    Element result = localScope[elementName];
+    if (result == null && isPatch) {
+      result = origin.localScope[elementName];
+    }
+    return result;
   }
 
   /**
@@ -515,7 +516,11 @@ class CompilationUnitElement extends ContainerElement {
     // Keep a list of top level members.
     super.addMember(element, listener);
     // Provide the member to the library to build scope.
-    getLibrary().addMember(element, listener);
+    if (enclosingElement.isPatch) {
+      getImplementationLibrary().addMember(element, listener);
+    } else {
+      getLibrary().addMember(element, listener);
+    }
   }
 }
 
@@ -539,6 +544,7 @@ class LibraryElement extends ScopeContainerElement {
   LibraryTag libraryTag;
   bool canUseNative = false;
   LibraryElement patch = null;
+  final LibraryElement origin;
 
   /**
    * Map for elements imported through import declarations.
@@ -548,15 +554,18 @@ class LibraryElement extends ScopeContainerElement {
    */
   final Map<SourceString, Element> importScope;
 
-  LibraryElement(Script script, [Uri uri])
+  LibraryElement(Script script, [Uri uri, LibraryElement this.origin])
     : this.uri = ((uri === null) ? script.uri : uri),
       importScope = new Map<SourceString, Element>(),
       super(new SourceString(script.name), ElementKind.LIBRARY, null) {
     entryCompilationUnit = new CompilationUnitElement(script, this);
+    if (isPatch) {
+      origin.patch = this;
+    }
   }
 
-
   bool get isPatched => patch !== null;
+  bool get isPatch => origin !== null;
 
   void addCompilationUnit(CompilationUnitElement element) {
     compilationUnits = compilationUnits.prepend(element);
@@ -586,6 +595,7 @@ class LibraryElement extends ScopeContainerElement {
     }
   }
 
+  LibraryElement getLibrary() => isPatch ? origin : this;
 
   /**
    * Look up a top-level element in this library. The element could
@@ -604,6 +614,8 @@ class LibraryElement extends ScopeContainerElement {
   /** Look up a top-level element in this library, but only look for
     * non-imported elements. Returns null if no such element exist. */
   Element findLocal(SourceString elementName) {
+    // TODO(johnniwinther): How to handle injected elements in the patch
+    // library?
     Element result = localScope[elementName];
     if (result === null || result.getLibrary() != this) return null;
     return result;
@@ -620,6 +632,19 @@ class LibraryElement extends ScopeContainerElement {
     });
   }
 
+  void forEachLocalMember(f(Element element),
+                          [includeInjectedMembers = false]) {

[ahe, 2012/09/24 12:30:18]

Could we get rid of includeInjectedMembers by using the implementation class?

[Johnni Winther, 2012/09/25 09:01:07]

Done.

+    localMembers.forEach(f);
+    if (includeInjectedMembers && patch != null) {
+      void filterPatch(Element element) {
+        if (!element.isPatch) {
+          f(element);
+        }
+      }
+      patch.forEachLocalMember(filterPatch);
+    }
+  }
+
   bool hasLibraryName() => libraryTag !== null;
 
   /**
@@ -637,9 +662,27 @@ class LibraryElement extends ScopeContainerElement {
     }
   }
 
-  Scope buildEnclosingScope() => new TopScope(this);
+  Scope buildEnclosingScope([bool patchScope = false]) {
+    if (origin !== null) {
+      return new PatchLibraryScope(origin, this);
+    } if (patchScope && patch !== null) {
+      return new PatchLibraryScope(this, patch);
+    } else {
+      return new TopScope(this);
+    }
+  }
 
   bool get isPlatformLibrary => uri.scheme == "dart";
+
+  String toString() {
+    if (origin !== null) {
+      return 'patch library(${getLibraryOrScriptName()})';
+    } else if (patch !== null) {
+      return 'origin library(${getLibraryOrScriptName()})';
+    } else {
+      return 'library(${getLibraryOrScriptName()})';
+    }
+  }
 }
 
 class PrefixElement extends Element {
@@ -694,9 +737,9 @@ class TypedefElement extends Element implements TypeDeclarationElement {
 
   Link<DartType> get typeVariables => cachedType.typeArguments;
 
-  Scope buildScope() =>
-      new TypeDeclarationScope(enclosingElement.buildScope(), this);
 
+  Scope buildScope([bool patchScope = false]) =>
+      new TypeDeclarationScope(enclosingElement.buildScope(patchScope), this);
   TypedefElement cloneTo(Element enclosing, DiagnosticListener listener) {
     TypedefElement result = new TypedefElement(name, enclosing);
     return result;
@@ -854,10 +897,10 @@ class VariableListElement extends Element {
     return isMember() && !modifiers.isStatic();
   }
 
-  Scope buildScope() {
-    Scope result = new VariableScope(enclosingElement.buildScope(), this);
+  Scope buildScope([bool patchScope = false]) {
+    Scope result = new VariableScope(enclosingElement.buildScope(patchScope), this);
     if (enclosingElement.isClass()) {
-      ClassScope clsScope = result.parent;
+      Scope clsScope = result.parent;
       clsScope.inStaticContext = !isInstanceMember();
     }
     return result;
@@ -988,6 +1031,7 @@ class FunctionElement extends Element {
    */
   // TODO(lrn): Consider using [defaultImplementation] to store the patch.
   FunctionElement patch = null;
+  FunctionElement origin = null;
 
   /**
    * If this is an interface constructor, [defaultImplementation] will
@@ -1027,6 +1071,7 @@ class FunctionElement extends Element {
   }
 
   bool get isPatched => patch !== null;
+  bool get isPatch => origin !== null;
 
   /**
    * Applies a patch function to this function. The patch function's body
@@ -1037,9 +1082,7 @@ class FunctionElement extends Element {
   void setPatch(FunctionElement patchElement) {
     // Sanity checks. The caller must check these things before calling.
     assert(patch === null);
-    assert(cachedNode === null);
     this.patch = patchElement;
-    cachedNode = patchElement.cachedNode;
   }
 
   bool isInstanceMember() {
@@ -1076,15 +1119,12 @@ class FunctionElement extends Element {
   }
 
   Node parseNode(DiagnosticListener listener) {
-    if (cachedNode !== null) return cachedNode;
     if (patch === null) {
       if (modifiers != null && modifiers.isExternal()) {
         listener.cancel("Compiling external function with no implementation.",
                         element: this);
       }
-      return null;
     }
-    cachedNode = patch.parseNode(listener);
     return cachedNode;
   }
 
@@ -1102,9 +1142,9 @@ class FunctionElement extends Element {
     }
   }
 
-  Scope buildScope() {
+  Scope buildScope([bool patchScope = false]) {
     Scope result =
-        new MethodScope(enclosingElement.buildScope(), this);
+        new MethodScope(enclosingElement.buildScope(patchScope), this);
     if (enclosingElement.isClass()) {
       Scope clsScope = result.parent;
       clsScope.inStaticContext = !isInstanceMember() && !isConstructor();
@@ -1230,6 +1270,7 @@ class ClassElement extends ScopeContainerElement
 
   // Lazily applied patch of class members.
   ClassElement patch = null;
+  ClassElement origin = null;
 
   ClassElement(SourceString name, Element enclosing, this.id, int initialState)
     : supertypeLoadState = initialState,
@@ -1238,15 +1279,20 @@ class ClassElement extends ScopeContainerElement
 
   InterfaceType computeType(compiler) {
     if (type == null) {
-      ClassNode node = parseNode(compiler);
-      Link<DartType> parameters =
-          TypeDeclarationElement.createTypeVariables(this, node.typeParameters);
-      type = new InterfaceType(this, parameters);
+      if (origin !== null) {
+        type = origin.computeType(compiler);
+      } else {
+        ClassNode node = parseNode(compiler);
+        Link<DartType> parameters =
+            TypeDeclarationElement.createTypeVariables(this, node.typeParameters);
+        type = new InterfaceType(this, parameters);
+      }
     }
     return type;
   }
 
   bool get isPatched => patch != null;
+  bool get isPatch => origin != null;
 
   /**
    * Return [:true:] if this element is the [:Object:] class for the [compiler].
@@ -1258,7 +1304,26 @@ class ClassElement extends ScopeContainerElement
 
   ClassElement ensureResolved(Compiler compiler) {
     if (resolutionState == STATE_NOT_STARTED) {
-      compiler.resolver.resolveClass(this);
+      if (origin !== null) {

[ahe, 2012/09/24 12:30:18]

I think this code should have been added to resolveClass in resolver. So this
method should have been left unchanged.

[Johnni Winther, 2012/09/25 09:01:07]

Done.

+        resolutionState = STATE_STARTED;
+        origin.ensureResolved(compiler);
+        computeType(compiler);
+        supertype = origin.supertype;
+        defaultClass = origin.defaultClass;
+        interfaces = origin.interfaces;
+        allSupertypes = origin.allSupertypes;
+        // Stepwise assignment to ensure invariant.
+        supertypeLoadState = STATE_STARTED;
+        supertypeLoadState = STATE_DONE;
+        resolutionState = STATE_DONE;
+        // TODO(johnniwinther): Check matching type variables and
+        // empty extends/implements clauses.
+      } else {
+        compiler.resolver.resolveClass(this);
+        if (patch !== null) {
+          patch.ensureResolved(compiler);
+        }
+      }
     }
     return this;
   }
@@ -1266,9 +1331,13 @@ class ClassElement extends ScopeContainerElement
   /**
    * Lookup local members in the class. This will ignore constructors.
    */
-  Element lookupLocalMember(SourceString memberName) {
+  Element lookupLocalMember(SourceString memberName,
+                            [bool includeInjectedMembers = false]) {

[ahe, 2012/09/24 12:30:18]

Get rid of includeInjectedMembers?

[Johnni Winther, 2012/09/25 09:01:07]

Done.

     var result = localLookup(memberName);
     if (result !== null && result.isConstructor()) return null;
+    if (result == null && patch != null && includeInjectedMembers) {
+      result = patch.localLookup(memberName);
+    }
     return result;
   }
 
@@ -1284,29 +1353,34 @@ class ClassElement extends ScopeContainerElement
    * This will ignore constructors.
    */
   Element lookupSuperMemberInLibrary(SourceString memberName,
-                                     LibraryElement library) {
+                                     LibraryElement library,
+                                     [includeInjectedMembers = false]) {
     bool isPrivate = memberName.isPrivate();
     for (ClassElement s = superclass; s != null; s = s.superclass) {
       // Private members from a different library are not visible.
       if (isPrivate && library !== s.getLibrary()) continue;
-      Element e = s.lookupLocalMember(memberName);
+      Element e = s.lookupLocalMember(memberName,
+          includeInjectedMembers: includeInjectedMembers);
       if (e === null) continue;
       // Static members are not inherited.
       if (e.modifiers.isStatic()) continue;
       return e;
     }
     if (isInterface()) {
-      return lookupSuperInterfaceMember(memberName, getLibrary());
+      return lookupSuperInterfaceMember(memberName, getLibrary(),
+          includeInjectedMembers: includeInjectedMembers);
     }
     return null;
   }
 
   Element lookupSuperInterfaceMember(SourceString memberName,
-                                     LibraryElement fromLibrary) {
+                                     LibraryElement fromLibrary,
+                                     [includeInjectedMembers = false]) {
     bool isPrivate = memberName.isPrivate();
     for (InterfaceType t in interfaces) {
       ClassElement cls = t.element;
-      Element e = cls.lookupLocalMember(memberName);
+      Element e = cls.lookupLocalMember(memberName,
+          includeInjectedMembers: includeInjectedMembers);
       if (e === null) continue;
       // Private members from a different library are not visible.
       if (isPrivate && fromLibrary !== e.getLibrary()) continue;
@@ -1327,12 +1401,14 @@ class ClassElement extends ScopeContainerElement
   Element lookupSelector(Selector selector) {
     SourceString memberName = selector.name;
     LibraryElement library = selector.library;
-    Element localMember = lookupLocalMember(memberName);
+    Element localMember = lookupLocalMember(memberName,
+                                            includeInjectedMembers: true);
     if (localMember != null &&
         (!memberName.isPrivate() || getLibrary() == library)) {
       return localMember;
     }
-    return lookupSuperMemberInLibrary(memberName, library);
+    return lookupSuperMemberInLibrary(memberName, library,
+                                      includeInjectedMembers: true);
   }
 
   /**
@@ -1405,9 +1481,10 @@ class ClassElement extends ScopeContainerElement
   Link<Element> get constructors {
     // TODO(ajohnsen): See if we can avoid this method at some point.
     Link<Element> result = const EmptyLink<Element>();
-    for (Element member in localMembers) {
+    // TODO(johnniwinther): Should we include injected constructors?
+    forEachMember((_, Element member) {
       if (member.isConstructor()) result = result.prepend(member);
-    }
+    });
     return result;
   }
 
@@ -1426,10 +1503,17 @@ class ClassElement extends ScopeContainerElement
    *
    * The enclosing class is passed to the callback. This is useful when
    * [includeSuperMembers] is [:true:].
+   *
+   * When [includeInjectedMembers] is [:true:] the members declared in patch
+   * classes are also included in the members for origin classes.
    */
+  // TODO(johnniwinther): Normalize member reporting for origin/patch classes:
+  // A call on a patch class such return the same as an origin class with
+  // [includeInjectedMembers] set to [:true:].
   void forEachMember([void f(ClassElement enclosingClass, Element member),
                       includeBackendMembers = false,
-                      includeSuperMembers = false]) {
+                      includeSuperMembers = false,
+                      includeInjectedMembers = false]) {
     Set<ClassElement> seen = new Set<ClassElement>();
     ClassElement classElement = this;
     do {
@@ -1448,6 +1532,20 @@ class ClassElement extends ScopeContainerElement
           f(classElement, element);
         }
       }
+      if (includeInjectedMembers) {
+        if (classElement.patch != null) {
+          void filterPatchMembers(patchClass, patchMember) {
+            if (!patchMember.isPatch) {
+              f(classElement, patchMember);
+            }
+          }
+          classElement.patch.forEachMember(
+              includeBackendMembers: includeBackendMembers,
+              includeSuperMembers: false,
+              includeInjectedMembers: false,
+              f: filterPatchMembers);
+        }
+      }
       classElement = includeSuperMembers ? classElement.superclass : null;
     } while(classElement !== null);
   }
@@ -1461,10 +1559,14 @@ class ClassElement extends ScopeContainerElement
    * When [includeBackendMembers] and [includeSuperMembers] are both [:true:]
    * then the fields are visited in the same order as they need to be given
    * to the JavaScript constructor.
+   *
+   * When [includeInjectedMembers] is [:true:] the fields declared in patch
+   * classes are also included in the fields for origin classes.
    */
   void forEachInstanceField([void f(ClassElement enclosingClass, Element field),
                              includeBackendMembers = false,
-                             includeSuperMembers = false]) {
+                             includeSuperMembers = false,
+                                 includeInjectedMembers = false]) {
     // Filters so that [f] is only invoked with instance fields.
     void fieldFilter(ClassElement enclosingClass, Element member) {
       if (member.isInstanceMember() && member.kind == ElementKind.FIELD) {
@@ -1472,7 +1574,8 @@ class ClassElement extends ScopeContainerElement
       }
     }
 
-    forEachMember(fieldFilter, includeBackendMembers, includeSuperMembers);
+    forEachMember(fieldFilter, includeBackendMembers,
+                  includeSuperMembers, includeInjectedMembers);
   }
 
   bool implementsInterface(ClassElement intrface) {
@@ -1503,8 +1606,25 @@ class ClassElement extends ScopeContainerElement
   bool isNative() => nativeName != null;
   int hashCode() => id;
 
-  Scope buildScope() =>
-      new ClassScope(enclosingElement.buildScope(), this);
+  Scope buildScope([bool patchScope = false]) {
+    if (origin !== null) {
+      return new PatchClassScope(enclosingElement.buildScope(patchScope),
+                                 origin, this);
+    } else if (patchScope && patch !== null) {
+      return new PatchClassScope(enclosingElement.buildScope(patchScope),
+                                 this, patch);
+    } else {
+      return new ClassScope(enclosingElement.buildScope(patchScope), this);
+    }
+  }
+
+  Scope buildLocalScope() {
+    if (origin !== null) {
+      return new LocalPatchClassScope(origin, this);
+    } else {
+      return new LocalClassScope(this);
+    }
+  }
 
   ClassElement cloneTo(Element enclosing, DiagnosticListener listener) {
     listener.internalErrorOnElement(this, 'unsupported operation');
@@ -1513,6 +1633,16 @@ class ClassElement extends ScopeContainerElement
   Link<DartType> get allSupertypesAndSelf {
     return allSupertypes.prepend(new InterfaceType(this));
   }
+
+  String toString() {
+    if (origin !== null) {
+      return 'patch ${super.toString()}';
+    } else if (patch !== null) {
+      return 'origin ${super.toString()}';
+    } else {
+      return super.toString();
+    }
+  }
 }
 
 class Elements {