Don't generate forwarding hooks if all noSuchMethod implementations

pkg/compiler/lib/src/js_backend/no_such_method_analysis.dart

Index: pkg/compiler/lib/src/js_backend/no_such_method_analysis.dart
diff --git a/pkg/compiler/lib/src/js_backend/no_such_method_analysis.dart b/pkg/compiler/lib/src/js_backend/no_such_method_analysis.dart
new file mode 100644
index 0000000000000000000000000000000000000000..3ed96bf8ce225b0ecb413169177b6a45807d5265
--- /dev/null
+++ b/pkg/compiler/lib/src/js_backend/no_such_method_analysis.dart
@@ -0,0 +1,144 @@
+// Copyright (c) 2015, 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.
+
+part of js_backend;
+
+/**
+ * Analyzes `noSuchMethod` implementations.
+ *
+ * If user code includes `noSuchMethod` implementations, type inference is
+ * hindered because (for instance) any selector where the type of the
+ * receiver is not known all implementations of `noSuchMethod` must be taken
+ * into account when inferring the return type.
+ *
+ * The situation can be ameliorated with some heuristics for disregarding some
+ * `noSuchMethod` implementations during type inference. We can partition
+ * `noSuchMethod` implementations into 3 categories.
+ *
+ * Implementations in category A have the form
+ *
+ *     noSuchMethod(x) => super.noSuchMethod(x);
+ *
+ * where the `super.noSuchMethod` hits `Object.noSuchMethod`,
+ * `Interceptor.noSuchMethod`, or another implementation in category A.
+ *
+ * Implementations in category B immediately throw, and all other
+ * implementations are in category C. A and B implementations may be safely
+ * ignored during type inference. After type inference, we may further partition
+ * C into C1 and C2, where C1 implementations have no return type (they are
+ * guaranteed to throw) and C2, those with return types.
+ */
+class NoSuchMethodAnalysis {

[floitsch, 2015/03/13 15:51:49]

I wouldn't call this "Analysis".
In the end it's a registry (or similar) that can answer some questions. It needs
to analyze the given nsms, but that's implicit.

[Harry Terkelsen, 2015/03/17 21:43:11]

Done.

+  /// The implementations that fall into category A, described above.
+  final Set<FunctionElement> forwardingImpls = new Set<FunctionElement>();
+  /// The implementations that fall into category B, described above.
+  final Set<FunctionElement> throwingImpls = new Set<FunctionElement>();
+  /// The implementations that fall into category C, described above.
+  final Set<Element> otherImpls = new Set<Element>();
+
+  /// The implementations that have not yet been categorized.
+  final Set<Element> uncategorizedImpls = new Set<Element>();
+
+  final JavaScriptBackend backend;
+  final Compiler compiler;
+
+  NoSuchMethodAnalysis(JavaScriptBackend backend)
+      : this.backend = backend,
+        this.compiler = backend.compiler;
+
+  void registerNoSuchMethod(Element noSuchMethodElement) {
+    uncategorizedImpls.add(noSuchMethodElement);
+  }
+
+  void onQueueEmpty(Enqueuer enqueuer) {
+    uncategorizedImpls.forEach(_categorizeImpl);
+    uncategorizedImpls.clear();
+    if (!compiler.enabledNoSuchMethod &&

[Johnni Winther, 2015/03/16 14:20:43]

Move this to the `CodegenEnqueuer.registerNoSuchMethod` where it should notify
the backend that a noSuchMethod of category A, B, or C has been enqueued.

[Harry Terkelsen, 2015/03/17 21:43:11]

See earlier comment about how the enqueuers and backend relate wrt noSuchMethod.
Please let me know if you think this is a good approach

+        (throwingImpls.isNotEmpty || otherImpls.isNotEmpty)) {
+      backend.enableNoSuchMethod(enqueuer);

[floitsch, 2015/03/13 15:51:49]

I would prefer if the "analysis" didn't have any side-effect.

[Harry Terkelsen, 2015/03/17 21:43:11]

Done.

+      compiler.enabledNoSuchMethod = true;
+    }
+  }
+
+  void _categorizeImpl(Element noSuchMethodElement) {
+    assert(noSuchMethodElement.name == Compiler.NO_SUCH_METHOD);
+    if (forwardingImpls.contains(noSuchMethodElement) ||
+        throwingImpls.contains(noSuchMethodElement) ||
+        otherImpls.contains(noSuchMethodElement)) return;
+    if (noSuchMethodElement is! FunctionElement ||
+        !compiler.noSuchMethodSelector.signatureApplies(noSuchMethodElement)) {
+      otherImpls.add(noSuchMethodElement);
+    }
+    FunctionElement noSuchMethodFunc = noSuchMethodElement as FunctionElement;
+    if (isForwarding(noSuchMethodFunc)) {
+      forwardingImpls.add(noSuchMethodFunc);
+    } else if (isThrowing(noSuchMethodFunc)) {
+      throwingImpls.add(noSuchMethodFunc);
+    } else {
+      otherImpls.add(noSuchMethodFunc);
+    }
+  }
+
+  bool isForwarding(FunctionElement element) {
+    if (backend.isDefaultNoSuchMethodImplementation(element)) {
+      return true;
+    }
+
+    if (!hasForwardingSyntax(element)) {
+      return false;
+    }
+
+    // Check if the super call hits a forwarding implementation
+    Element superCall = element.enclosingClass
+        .lookupSuperSelector(compiler.noSuchMethodSelector);
+    _categorizeImpl(superCall);

[Johnni Winther, 2015/03/16 14:20:43]

Maybe [_categorizeImpl] should return an enum result which we can check
directly.

[Harry Terkelsen, 2015/03/17 21:55:16]

Done.

+    return forwardingImpls.contains(superCall);
+  }
+
+  bool hasForwardingSyntax(FunctionElement element) {
+    String param = element.parameters.single.name;
+    Statement body = element.node.body;
+    Expression expr;
+    if (body is Return && body.isArrowBody) {
+      expr = body.expression;
+    } else if (body is Block &&
+        !body.statements.isEmpty &&
+        body.statements.nodes.tail.isEmpty) {
+      Statement stmt = body.statements.nodes.head;
+      if (stmt is Return && stmt.hasExpression) {
+        expr = stmt.expression;
+      }
+    }
+    if (expr is Send &&
+        expr.isSuperCall &&
+        expr.selector is Identifier &&
+        (expr.selector as Identifier).source == Compiler.NO_SUCH_METHOD) {
+      var arg = expr.arguments.head;
+      if (arg is Send &&
+          arg.argumentsNode == null &&
+          arg.receiver == null &&
+          arg.selector is Identifier &&
+          arg.selector.source == param) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  bool isThrowing(FunctionElement element) {
+    Statement body = element.node.body;
+    if (body is Return && body.isArrowBody) {
+      if (body.expression is Throw) {
+        return true;
+      }
+    } else if (body is Block &&
+        !body.statements.isEmpty &&
+        body.statements.nodes.tail.isEmpty) {
+      if (body.statements.nodes.head is Throw) {
+        return true;
+      }
+    }
+    return false;
+  }
+}