Patch signatures checked in resolver.

lib/compiler/implementation/resolver.dart

Index: lib/compiler/implementation/resolver.dart
diff --git a/lib/compiler/implementation/resolver.dart b/lib/compiler/implementation/resolver.dart
index 91144c07a22cd284f7bdfb17509ae102bff710a9..b5ded2a42246ce5950d31dc542f10fb8ada5e338 100644
--- a/lib/compiler/implementation/resolver.dart
+++ b/lib/compiler/implementation/resolver.dart
@@ -133,9 +133,74 @@ class ResolverTask extends CompilerTask {
     }
   }
 
+  void checkMatchingPatchParameters(FunctionElement origin,
+                                    Link<Element> originParameters,
+                                    Link<Element> patchParameters) {
+    while (!originParameters.isEmpty()) {
+      Element originParameter = originParameters.head;
+      Element patchParameter = patchParameters.head;
+      // Hack: Use unparser to test parameter equality. This only works because
+      // we are restricting patch uses and the approach cannot be used

[Lasse Reichstein Nielsen, 2012/10/04 07:51:21]

How are we restricting it?
And isn't this incredibly inefficient?

[Johnni Winther, 2012/10/04 10:23:29]

Restrictions: See the next comment.
Inefficient: Yes, but parameter node trees are generally small.

+      // elsewhere.
+      String originParameterText =
+          originParameter.parseNode(compiler).toString();
+      String patchParameterText =
+          patchParameter.parseNode(compiler).toString();
+      if (originParameterText != patchParameterText) {

[Lasse Reichstein Nielsen, 2012/10/04 07:51:21]

Does this require that the positional parameters have the same name? It's
reasonable, but not really necessary.

[Johnni Winther, 2012/10/04 10:23:29]

Yes, parameters must have the same name, and we do not even allow a constructor
implementation to use this.field. We can implement patches without this
flexibility, so the cost of writing a more liberal equality for parameters
seemed to high.

+        error(originParameter.parseNode(compiler),
+            MessageKind.PATCH_PARAMETER_MISMATCH,
+            [origin.name, originParameterText, patchParameterText]);
+      }
+
+      originParameters = originParameters.tail;
+      patchParameters = patchParameters.tail;
+    }
+  }
+
   void checkMatchingPatchSignatures(FunctionElement origin,
                                     FunctionElement patch) {
-    // TODO(johnniwinther): Stub. Implementation in a later CL.
+    // TODO(johnniwinther): Show both origin and patch locations on errors.
+    FunctionExpression originTree = compiler.withCurrentElement(origin, () {
+      return origin.parseNode(compiler);
+    });
+    FunctionSignature originSignature = compiler.withCurrentElement(origin, () {
+      return origin.computeSignature(compiler);
+    });
+    FunctionExpression patchTree = compiler.withCurrentElement(patch, () {
+      return patch.parseNode(compiler);
+    });
+    FunctionSignature patchSignature = compiler.withCurrentElement(patch, () {
+      return patch.computeSignature(compiler);
+    });
+

[Lasse Reichstein Nielsen, 2012/10/04 07:51:21]

Why is there not a == on FunctionSignature that does all this?

[Johnni Winther, 2012/10/04 10:23:29]

It would require the aforementioned equality on parameters.

+    if (originSignature.returnType != patchSignature.returnType) {
+      Node errorNode =
+          originTree.returnType !== null ? originTree.returnType : originTree;

[ahe, 2012/10/04 08:34:21]

I think the errorNode should be derived from patchTree.

[Johnni Winther, 2012/10/04 10:23:29]

Done.

+      error(errorNode, MessageKind.PATCH_RETURN_TYPE_MISMATCH,
+          [origin.name, originSignature.returnType, patchSignature.returnType]);
+    }
+    if (originSignature.requiredParameterCount !=
+        patchSignature.requiredParameterCount) {
+      error(originTree,

[ahe, 2012/10/04 08:34:21]

patchTree

[Johnni Winther, 2012/10/04 10:23:29]

Done.

+            MessageKind.PATCH_REQUIRED_PARAMETER_COUNT_MISMATCH,
+            [origin.name, originSignature.requiredParameterCount,
+             patchSignature.requiredParameterCount]);
+    } else {
+      checkMatchingPatchParameters(origin,
+                                   originSignature.requiredParameters,
+                                   patchSignature.requiredParameters);
+    }
+    if (originSignature.optionalParameterCount !=
+        patchSignature.optionalParameterCount) {
+      error(originTree,
+            MessageKind.PATCH_OPTIONAL_PARAMETER_COUNT_MISMATCH,
+            [origin.name, originSignature.optionalParameterCount,
+             patchSignature.optionalParameterCount]);
+    } else {
+      checkMatchingPatchParameters(origin,
+                                   originSignature.optionalParameters,
+                                   patchSignature.optionalParameters);

[Lasse Reichstein Nielsen, 2012/10/04 07:51:21]

Does this handle both optional positional parameters and optional named
parameters?

[Johnni Winther, 2012/10/04 10:23:29]

Yes, but a check is added to ensure that both are either named or positional.

+    }
   }
 
   TreeElements resolveMethodElement(FunctionElement element) {