Patch signatures checked in resolver.

lib/compiler/implementation/resolver.dart

 // Copyright (c) 2012, 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.
 
 abstract class TreeElements {
   Element operator[](Node node);
   Selector getSelector(Send send);
   DartType getType(TypeAnnotation annotation);
   bool isParameterChecked(Element element);
 }
@@ skipping 115 matching lines @@
     while (redirection !== null) {
       if (seen.contains(redirection)) {
         resolver.visitor.error(node, MessageKind.REDIRECTING_CONSTRUCTOR_CYCLE);
         return;
       }
       seen.add(redirection);
       redirection = resolveConstructorRedirection(redirection);
     }
   }
 
+  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) {
     assert(invariant(element, element.isDeclaration));
     return compiler.withCurrentElement(element, () {
       bool isConstructor = element.kind === ElementKind.GENERATIVE_CONSTRUCTOR;
       TreeElements elements =
           compiler.enqueuer.resolution.getCachedElements(element);
       if (elements !== null) {
         assert(isConstructor);
@@ skipping 2898 matching lines @@
     return result;
   }
   Element lookup(SourceString name) => localLookup(name);
   Element lexicalLookup(SourceString name) => localLookup(name);
 
   Element add(Element newElement) {
     throw "Cannot add an element in a patch library scope";
   }
   String toString() => 'PatchLibraryScope($origin,$patch)';
 }