From 6dc48ab999640fab5e0e01f7e09f69ea8d7dd39b Mon Sep 17 00:00:00 2001
From: Vijay Menon <vsm@google.com>
Date: Wed, 4 Dec 2019 23:19:26 +0000
Subject: [PATCH] [dartdevc] Destructure optional positional parameters

Use ES6 destructuring to handle optional positional arguments more compactly.
Note, this doesn't actually change calling convention in DDC, just how
optionals are handled at the callee.

Given Dart:
  void foo(arg1, arg2, [opt1, opt2 = def2]) {
    ...
  }
old JS:
  function foo(arg1, arg2, opt1, opt2) {
    if (opt1 === void 0) opt1 = null;
    if (opt2 === void 0) opt2 = 42;
    ...
  }
new JS:
  function foo(arg1, arg2, opt1 = null, opt2 = def2) {
    ...
  }

We should be able to similar with named params.

Change-Id: I8491a4517d729ab1dec40c1ed2073b9c33cdffe0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/124107
Reviewed-by: Mark Zhou <markzipan@google.com>
Commit-Queue: Vijay Menon <vsm@google.com>
---
 pkg/dev_compiler/lib/src/kernel/compiler.dart | 38 ++++++++++++-------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/pkg/dev_compiler/lib/src/kernel/compiler.dart b/pkg/dev_compiler/lib/src/kernel/compiler.dart
index 879c848aaf5e..6357defb905b 100644
--- a/pkg/dev_compiler/lib/src/kernel/compiler.dart
+++ b/pkg/dev_compiler/lib/src/kernel/compiler.dart
@@ -814,7 +814,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
       for (var ctor in superclass.constructors) {
         var savedUri = _currentUri;
         _currentUri = ctor.enclosingClass.fileUri;
-        var jsParams = _emitParameters(ctor.function);
+        var jsParams = _emitParameters(ctor.function, isForwarding: true);
         _currentUri = savedUri;
         var name = ctor.name.name;
         var ctorBody = [
@@ -2817,9 +2817,28 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     return js_ast.Fun(formals, block);
   }
 
-  List<js_ast.Parameter> _emitParameters(FunctionNode f) {
+  js_ast.Parameter _emitParameter(VariableDeclaration node,
+      {bool withoutInitializer = false}) {
+    var initializer = node.initializer;
+    var id = _emitVariableDef(node);
+    if (initializer == null || withoutInitializer) return id;
+    return js_ast.DestructuredVariable(
+        name: id, defaultValue: _visitExpression(initializer));
+  }
+
+  List<js_ast.Parameter> _emitParameters(FunctionNode f,
+      {bool isForwarding = false}) {
+    // Destructure optional positional parameters in place.
+    // Given:
+    //  - (arg1, arg2, [opt1, opt2 = def2])
+    // Emit:
+    //  - (arg1, arg2, opt1 = null, opt2 = def2)
+    // Note, if [isForwarding] is set, omit initializers as this actually a
+    // forwarded call not a parameter list. E.g., the second in:
+    //  - foo(arg1, opt1 = def1) => super(arg1, opt1).
     var positional = f.positionalParameters;
-    var result = List<js_ast.Parameter>.of(positional.map(_emitVariableDef));
+    var result = List<js_ast.Parameter>.of(positional
+        .map((p) => _emitParameter(p, withoutInitializer: isForwarding)));
     if (positional.isNotEmpty &&
         f.requiredParameterCount == positional.length &&
         positional.last.annotations.any(isJsRestAnnotation)) {
@@ -2910,7 +2929,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
       //
       // In the future, we might be able to simplify this, see:
       // https://github.com/dart-lang/sdk/issues/28320
-      var jsParams = _emitParameters(function);
+      var jsParams = _emitParameters(function, isForwarding: true);
       var mutatedParams = jsParams;
       var gen = emitGeneratorFn((fnBody) {
         var mutatedVars = js_ast.findMutatedVariables(fnBody);
@@ -3051,19 +3070,10 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
       }
     }
 
-    for (var p in f.positionalParameters.take(f.requiredParameterCount)) {
+    for (var p in f.positionalParameters) {
       var jsParam = _emitIdentifier(p.name);
       initParameter(p, jsParam);
     }
-    for (var p in f.positionalParameters.skip(f.requiredParameterCount)) {
-      var jsParam = _emitIdentifier(p.name);
-      var defaultValue = _defaultParamValue(p);
-      if (defaultValue != null) {
-        body.add(js.statement(
-            'if (# === void 0) # = #;', [jsParam, jsParam, defaultValue]));
-      }
-      initParameter(p, jsParam);
-    }
     for (var p in f.namedParameters) {
       // Parameters will be passed using their real names, not the (possibly
       // renamed) local variable.