Insert default arguments for static calls and constructor calls.

pkg/compiler/lib/src/ssa/builder_kernel.dart

 // Copyright (c) 2016, 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.
 
 import 'package:kernel/ast.dart' as ir;
 
 import '../common.dart';
 import '../common/codegen.dart' show CodegenRegistry, CodegenWorkItem;
 import '../common/names.dart';
 import '../common/tasks.dart' show CompilerTask;
@@ skipping 1053 matching lines @@
   @override
   void visitLet(ir.Let let) {
     ir.VariableDeclaration variable = let.variable;
     variable.initializer.accept(this);
     HInstruction initializedValue = pop();
     // TODO(sra): Apply inferred type information.
     letBindings[variable] = initializedValue;
     let.body.accept(this);
   }
 
-  // TODO(het): Also extract type arguments
-  /// Extracts the list of instructions for the expressions in the arguments.
-  List<HInstruction> _visitArguments(ir.Arguments arguments) {
+  /// Extracts the list of instructions for the expressions in the list.
+  List<HInstruction> _visitList(List<ir.Expression> expressions) {
     List<HInstruction> result = <HInstruction>[];
-
-    for (ir.Expression argument in arguments.positional) {
-      argument.accept(this);
+    for (ir.Expression expression in expressions) {
+      expression.accept(this);
       result.add(pop());
     }
-    for (ir.NamedExpression argument in arguments.named) {
-      argument.value.accept(this);
-      result.add(pop());
-    }
-
     return result;
   }
 
+  /// Builds the list of instructions for the expressions in the arguments to a
+  /// dynamic target (member function).

[Siggi Cherem (dart-lang), 2016/12/06 18:04:15]

now might be a good opportunity to document why this and static targets look
different. In particular, we can say that for dynamic targets we generate stubs
that will fill-in default values for non-specified optional arguments, but that
with static targets we just fill them in directly at the call site.

[sra1, 2016/12/06 18:27:37]

Done.

+  List<HInstruction> _visitArgumentsForDynamicTarget(
+      Selector selector, ir.Arguments arguments) {
+    List<HInstruction> values = _visitList(arguments.positional);
+
+    if (arguments.named.isEmpty) return values;
+
+    var namedValues = <String, HInstruction>{};
+    for (ir.NamedExpression argument in arguments.named) {
+      argument.value.accept(this);
+      namedValues[argument.name] = pop();
+    }
+    for (String name in selector.callStructure.getOrderedNamedArguments()) {
+      values.add(namedValues[name]);
+    }
+
+    return values;
+  }
+
+  /// Build argument list in canonical order for a static [target], including
+  /// defaulted arguments.
+  List<HInstruction> _visitArgumentsForStaticTarget(
+      ir.FunctionNode target, ir.Arguments arguments) {
+    // Visit arguments in source order, then re-order and fill in defaults.
+    var values = _visitList(arguments.positional);
+
+    while (values.length < target.positionalParameters.length) {
+      ir.VariableDeclaration parameter =
+          target.positionalParameters[values.length];
+      values.add(_defaultValueForParameter(parameter));
+    }
+
+    if (arguments.named.isEmpty) return values;
+
+    var namedValues = <String, HInstruction>{};
+    for (ir.NamedExpression argument in arguments.named) {
+      argument.value.accept(this);
+      namedValues[argument.name] = pop();
+    }
+
+    // Visit named arguments in parameter-position order, selecting provided or
+    // default value.
+    // TODO(sra): Ensure the stored order is canonical so we don't have to
+    // sort. The old builder uses CallStructure.makeArgumentList which depends
+    // on the old element model.
+    var namedParameters = target.namedParameters.toList()
+      ..sort((ir.VariableDeclaration a, ir.VariableDeclaration b) =>
+          a.name.compareTo(b.name));
+    for (ir.VariableDeclaration parameter in namedParameters) {
+      HInstruction value = namedValues[parameter.name];
+      if (value == null) {
+        values.add(_defaultValueForParameter(parameter));
+      } else {
+        values.add(value);
+        namedValues.remove(parameter.name);
+      }
+    }
+    assert(namedValues.isEmpty);
+
+    return values;
+  }
+
+  HInstruction _defaultValueForParameter(ir.VariableDeclaration parameter) {
+    ir.Expression initializer = parameter.initializer;
+    if (initializer == null) return graph.addConstantNull(compiler);
+    // TODO(sra): Evaluate constant in ir.Node domain.

[Siggi Cherem (dart-lang), 2016/12/06 18:04:15]

when we do - I'd like to request from kernel-ir some invariants. For example,
that there are no invalid nodes and we can use a very simple visitor to do the
evaluation.

[sra1, 2016/12/06 18:27:37]

Acknowledged.

+    ConstantValue constant =
+        astAdapter.getConstantForParameterDefaultValue(initializer);
+    if (constant == null) return graph.addConstantNull(compiler);
+    return graph.addConstant(constant, compiler);
+  }
+
   @override
   void visitStaticInvocation(ir.StaticInvocation invocation) {
     ir.Procedure target = invocation.target;
     if (astAdapter.isInForeignLibrary(target)) {
       handleInvokeStaticForeign(invocation, target);
       return;
     }
     TypeMask typeMask = astAdapter.returnTypeOf(target);
 
-    List<HInstruction> arguments = _visitArguments(invocation.arguments);
+    // TODO(sra): For JS interop external functions, use a different function to
+    // build arguments.
+    List<HInstruction> arguments =
+        _visitArgumentsForStaticTarget(target.function, invocation.arguments);
 
     _pushStaticInvocation(target, arguments, typeMask);
   }
 
   void handleInvokeStaticForeign(
       ir.StaticInvocation invocation, ir.Procedure target) {
     String name = target.name.name;
     if (name == 'JS') {
       handleForeignJs(invocation);
     } else if (name == 'JS_CURRENT_ISOLATE_CONTEXT') {
@@ skipping 126 matching lines @@
       _pushStaticInvocation(target, <HInstruction>[], backend.dynamicType);
     }
   }
 
   void handleForeignJsCallInIsolate(ir.StaticInvocation invocation) {
     if (_unexpectedForeignArguments(invocation, 2, 2)) {
       stack.add(graph.addConstantNull(compiler)); // Result expected on stack.
       return;
     }
 
-    List<HInstruction> inputs = _visitArguments(invocation.arguments);
+    List<HInstruction> inputs = _visitList(invocation.arguments.positional);

[Johnni Winther, 2016/12/06 08:36:37]

I'd prefer calling [_visitArgumentsForStaticTarget] here, or add new helper
methods for simple arguments that asserts that there are no optional arguments.

[sra1, 2016/12/06 18:27:37]

The _unexpectedForeignArguments call ensures there are no named optional
arguments.
I renamed _visitList to _visitPositionalArguments and passed the ir.Arguments

 
     if (!compiler.hasIsolateSupport) {
       // If the isolate library is not used, we ignore the isolate argument and
       // just invoke the closure.
       push(new HInvokeClosure(new Selector.callClosure(0),
           <HInstruction>[inputs[1]], backend.dynamicType));
     } else {
       // Call a helper method from the isolate library.
       ir.Procedure callInIsolate = astAdapter.callInIsolate;
       if (callInIsolate == null) {
@@ skipping 49 matching lines @@
     stack.add(graph.addConstantNull(compiler)); // Result expected on stack.
     return;
   }
 
   void handleForeignJsSetStaticState(ir.StaticInvocation invocation) {
     if (_unexpectedForeignArguments(invocation, 1, 1)) {
       stack.add(graph.addConstantNull(compiler)); // Result expected on stack.
       return;
     }
 
-    List<HInstruction> inputs = _visitArguments(invocation.arguments);
+    List<HInstruction> inputs = _visitList(invocation.arguments.positional);
 
     String isolateName = backend.namer.staticStateHolder;
     SideEffects sideEffects = new SideEffects.empty();
     sideEffects.setAllSideEffects();
-    push(new HForeignCode(js.js.parseForeignJS("$isolateName = #"),
-        backend.dynamicType, inputs,
+    push(new HForeignCode(
+        js.js.parseForeignJS("$isolateName = #"), backend.dynamicType, inputs,
         nativeBehavior: native.NativeBehavior.CHANGES_OTHER,
         effects: sideEffects));
   }
 
   void handleForeignJsGetStaticState(ir.StaticInvocation invocation) {
     if (_unexpectedForeignArguments(invocation, 0, 0)) {
       stack.add(graph.addConstantNull(compiler)); // Result expected on stack.
       return;
     }
 
@@ skipping 177 matching lines @@
         isStatement: !nativeBehavior.codeTemplate.isExpression,
         effects: nativeBehavior.sideEffects,
         nativeBehavior: nativeBehavior)..sourceInformation = sourceInformation);
   }
 
   void handleJsStringConcat(ir.StaticInvocation invocation) {
     if (_unexpectedForeignArguments(invocation, 2, 2)) {
       stack.add(graph.addConstantNull(compiler)); // Result expected on stack.
       return;
     }
-    List<HInstruction> inputs = _visitArguments(invocation.arguments);
+    List<HInstruction> inputs = _visitList(invocation.arguments.positional);
     push(new HStringConcat(inputs[0], inputs[1], backend.stringType));
   }
 
   void _pushStaticInvocation(
       ir.Node target, List<HInstruction> arguments, TypeMask typeMask) {
     HInvokeStatic instruction = new HInvokeStatic(
         astAdapter.getMember(target), arguments, typeMask,
         targetCanThrow: astAdapter.getCanThrow(target));
     if (currentImplicitInstantiations.isNotEmpty) {
       instruction.instantiatedTypes =
@@ skipping 31 matching lines @@
   }
 
   // TODO(het): Decide when to inline
   @override
   void visitMethodInvocation(ir.MethodInvocation invocation) {
     // Handle `x == null` specially. When these come from null-aware operators,
     // there is no mapping in the astAdapter.
     if (_handleEqualsNull(invocation)) return;
     invocation.receiver.accept(this);
     HInstruction receiver = pop();
-
+    Selector selector = astAdapter.getSelector(invocation);
     _pushDynamicInvocation(
         invocation,
         astAdapter.typeOfInvocation(invocation),
         <HInstruction>[receiver]
-          ..addAll(_visitArguments(invocation.arguments)));
+          ..addAll(
+              _visitArgumentsForDynamicTarget(selector, invocation.arguments)));
   }
 
   bool _handleEqualsNull(ir.MethodInvocation invocation) {
     if (invocation.name.name == '==') {
       ir.Arguments arguments = invocation.arguments;
       if (arguments.types.isEmpty &&
           arguments.positional.length == 1 &&
           arguments.named.isEmpty) {
         bool finish(ir.Expression comparand) {
           comparand.accept(this);
@@ skipping 20 matching lines @@
   static ir.Class _containingClass(ir.TreeNode node) {
     while (node != null) {
       if (node is ir.Class) return node;
       node = node.parent;
     }
     return null;
   }
 
   @override
   void visitSuperMethodInvocation(ir.SuperMethodInvocation invocation) {
-    List<HInstruction> arguments = _visitArguments(invocation.arguments);
+    Selector selector = astAdapter.getSelector(invocation);
+    List<HInstruction> arguments = _visitArgumentsForStaticTarget(
+        invocation.interfaceTarget.function, invocation.arguments);
     HInstruction receiver = localsHandler.readThis();
-    Selector selector = astAdapter.getSelector(invocation);
     ir.Class surroundingClass = _containingClass(invocation);
 
     List<HInstruction> inputs = <HInstruction>[];
     if (astAdapter.isIntercepted(invocation)) {
       inputs.add(_interceptorFor(receiver));
     }
     inputs.add(receiver);
     inputs.addAll(arguments);
 
     HInstruction instruction = new HInvokeSuper(
         astAdapter.getMethod(invocation.interfaceTarget),
         astAdapter.getClass(surroundingClass),
         selector,
         inputs,
         astAdapter.returnTypeOf(invocation.interfaceTarget),
         null,
         isSetter: selector.isSetter || selector.isIndexSet);
     instruction.sideEffects =
         compiler.closedWorld.getSideEffectsOfSelector(selector, null);
     push(instruction);
   }
 
   @override
   void visitConstructorInvocation(ir.ConstructorInvocation invocation) {
     ir.Constructor target = invocation.target;
-    List<HInstruction> arguments = _visitArguments(invocation.arguments);
+    // TODO(sra): For JS-interop targets, process arguments differently.
+    List<HInstruction> arguments =
+        _visitArgumentsForStaticTarget(target.function, invocation.arguments);
     TypeMask typeMask = new TypeMask.nonNullExact(
         astAdapter.getElement(target.enclosingClass), compiler.closedWorld);
     _pushStaticInvocation(target, arguments, typeMask);
   }
 
   @override
   void visitIsExpression(ir.IsExpression isExpression) {
     isExpression.operand.accept(this);
     HInstruction expression = pop();
 
@@ skipping 67 matching lines @@
     push(new HNot(popBoolified(), backend.boolType));
   }
 
   @override
   void visitStringConcatenation(ir.StringConcatenation stringConcat) {
     KernelStringBuilder stringBuilder = new KernelStringBuilder(this);
     stringConcat.accept(stringBuilder);
     stack.add(stringBuilder.result);
   }
 }