Recognize "patch" identifier in .dartp files.

lib/compiler/implementation/compiler.dart

Index: lib/compiler/implementation/compiler.dart
diff --git a/lib/compiler/implementation/compiler.dart b/lib/compiler/implementation/compiler.dart
index 7dd7384e3f11ed35aa005e8fbfc20e8394551259..f160454430bee7a8531745d01ffa9f1ac82b5f6e 100644
--- a/lib/compiler/implementation/compiler.dart
+++ b/lib/compiler/implementation/compiler.dart
@@ -284,6 +284,7 @@ class Compiler implements DiagnosticListener {
   ScannerTask scanner;
   DietParserTask dietParser;
   ParserTask parser;
+  PatchParserTask patchParser;
   TreeValidatorTask validator;
   ResolverTask resolver;
   TypeCheckerTask checker;
@@ -323,6 +324,7 @@ class Compiler implements DiagnosticListener {
     scanner = new ScannerTask(this);
     dietParser = new DietParserTask(this);
     parser = new ParserTask(this);
+    patchParser = new PatchParserTask(this);
     validator = new TreeValidatorTask(this);
     resolver = new ResolverTask(this);
     checker = new TypeCheckerTask(this);
@@ -520,67 +522,122 @@ class Compiler implements DiagnosticListener {
     if (library.isPatched) return;
     Uri patchUri = resolvePatchUri(dartLibraryPath);
     if (patchUri !== null) {
-      // TODO(lrn): Use a different parser to allow for "patch" annotations.
-      // For now just assume everything in the patch library is a patch
-      // function.
-      LibraryElement patchLibrary = scanner.loadLibrary(patchUri, null);
+      LibraryElement patchLibrary =
+          patchParser.loadPatchLibrary(patchUri);
+      // We allow foreign functions in patched libraries.
+      addForeignFunctions(library);  // Is safe even if already added.
       applyLibraryPatch(library, patchLibrary);
     }
   }
 
-  void applyLibraryPatch(LibraryElement library, LibraryElement patch) {
-    // We allow foreign functions in patched libraries.
-    addForeignFunctions(library);  // Is safe even if already added.
+  void applyLibraryPatch(LibraryElement original, LibraryElement patch) {
+    Link<Element> patches = patch.topLevelElements;
 
     // Copy/patch top-level elements.
-    Link<Element> patches = patch.topLevelElements;
     while (!patches.isEmpty()) {
       Element patchElement = patches.head;
-      Element originalElement = library.elements[patchElement.name];
-      if (originalElement !== null) {
-        // Assume that we are patching if the original exists.
-        if (originalElement is! FunctionElement) {
-          // TODO(lrn): Handle class declarations too.
-          internalError("Can only patch functions", element: originalElement);
-        }
-        // TODO(lrn): Abort if the original isn't marked external, when
-        // that is added to the language.
-        if (patchElement is! FunctionElement ||
-            !patchSignatureMatches(originalElement, patchElement)) {
-          internalError("Can only patch functions with matching signatures",
+      Element originalElement = original.elements[patchElement.name];
+      // Getters and setters are kept inside a synthetic field.
+      if (patchElement.kind === ElementKind.ABSTRACT_FIELD) {
+        if (originalElement !== null &&
+            originalElement.kind !== ElementKind.ABSTRACT_FIELD) {
+          internalError("Cannot patch non-getter/setter with getter/setter",
                         element: originalElement);
         }
-        applyFunctionPatch(originalElement, patchElement);
+        AbstractFieldElement patchField = patchElement;
+        AbstractFieldElement originalField = originalElement;
+        if (patchField.getter !== null) {
+          if (originalField === null || originalField.getter === null) {
+            original.addGetterOrSetter(clonePatch(patchField.getter));
+          } else {
+            patchMember(originalField.getter, patchField.getter);
+          }
+        }
+        if (patchField.setter !== null) {
+          if (originalField === null || originalField.setter === null) {
+            original.addGetterOrSetter(clonePatch(patchField.setter));
+          } else {
+            patchMember(originalField.setter, patchField.setter);
+          }
+        }
+      } else if (originalElement === null) {
+        original.addMember(clonePatch(patchElement));
       } else {
-        // TODO(lrn): Allow adding private elements to the original library.
+        patchMember(originalElement, patchElement);
       }
       patches = patches.tail;
     }
 
     // Copy imports.
     Map<String, LibraryElement> delayedPatches = <LibraryElement>{};
-    Uri base = patch.script.uri;
+    Uri patchBase = patch.script.uri;
     for (ScriptTag tag in patch.tags.reverse()) {
       if (tag.isImport()) {
         StringNode argument = tag.argument;
-        Uri resolved = base.resolve(argument.dartString.slowToString());
+        Uri resolved = patchBase.resolve(argument.dartString.slowToString());
         LibraryElement importedLibrary =
             scanner.loadLibrary(resolved, argument);
-        scanner.importLibrary(library, importedLibrary, tag, patch);
+        scanner.importLibrary(original, importedLibrary, tag, patch);
         if (resolved.scheme == "dart") {
           delayedPatches[resolved.path] = importedLibrary;
         }
       }
     }
+
     // Mark library as already patched.
-    library.patch = patch;
-    // We patch imported libraries after marking the current library as patched,
-    // to avoid problems with cyclic dependencies.
+    original.patch = patch;
+
+    // We patch imported libraries after marking the current library as
+    // patched, to avoid problems with cyclic dependencies.
     delayedPatches.forEach((String path, LibraryElement importedLibrary) {
       patchDartLibrary(importedLibrary, path);
     });
   }
 
+  Element clonePatch(Element patchElement) {
+    bool isPatch(Element element) {
+      // TODO(lrn): More checks needed if we introduce matadata for real.

[Johnni Winther, 2012/07/06 07:57:39]

matadata -> metadata

[Lasse Reichstein Nielsen, 2012/07/06 10:26:23]

Done.

+      return !element.metadata.isEmpty();
+    }
+    if (isPatch(patchElement)) {
+      internalError("Cannot patch non-existing member '"
+                      "${patchElement.name.slowToString()}'.");

[Johnni Winther, 2012/07/06 07:57:39]

I can't see the relation between 1) the element having a non-empty metadata, 2)
it being a patch, AND 3) being a non-existing member. Why does 1) imply 2) and
3)?

[Lasse Reichstein Nielsen, 2012/07/06 10:26:23]

This function is for copying a declaration from the patch library to the patchee
library, which mustn't have a declaration of that name yet. The patch
declaration must not be marked as "patch", because then it's intended to replace
an existing element. I've added a more elaborate comment to explain that.
I'll move the test, so we don't get here with a patch-patch declaration.

+
+    }
+    if (!patchElement.name.isPrivate()) {
+      internalError("Cannot add non-private member '"
+                    "${patchElement.name.slowToString()}' from patch.");
+    }
+    // TODO(lrn): Create a copy of patchElement that isn't added to anything,
+    // but which takes its source from patchElement.
+    throw "Adding members from patch is unsupported";
+  }
+
+  void patchMember(Element originalElement,
+                   Element patchElement) {
+    // originalElement isn't null.
+    bool isPatch(Element element) {

[Johnni Winther, 2012/07/06 07:57:39]

Make a separate isPatch check to be used both in clonePatch and patchMember to
avoid redundancy.

[Lasse Reichstein Nielsen, 2012/07/06 10:26:23]

Done. I consider moving everything to patch-parser to keep the related functions
together, and not infect Compiler with too many methods.

+      // TODO(lrn): More checks needed if we introduce matadata for real.
+      return !element.metadata.isEmpty();
+    }
+    if (!isPatch(patchElement)) {
+      internalError("Cannot overwrite existing '"
+                    "${originalElement.name.slowToString()}' with non-patch.");
+    }
+    if (originalElement is! FunctionElement) {
+      // TODO(lrn): Handle class declarations too.
+      internalError("Can only patch functions", element: originalElement);
+    }
+    // TODO(lrn): Abort if the original isn't marked external, when
+    // that is added to the language.
+    if (patchElement is! FunctionElement ||
+        !patchSignatureMatches(originalElement, patchElement)) {
+      internalError("Can only patch functions with matching signatures",
+                    element: originalElement);
+    }
+    applyFunctionPatch(originalElement, patchElement);
+  }
+
   bool patchSignatureMatches(FunctionElement original, FunctionElement patch) {
     // TODO(lrn): Check that patches actually match the signature of
     // the function it's patching.