Support argument definition test in constructors.

lib/compiler/implementation/ssa/builder.dart

Index: lib/compiler/implementation/ssa/builder.dart
===================================================================
--- lib/compiler/implementation/ssa/builder.dart	(revision 14192)
+++ lib/compiler/implementation/ssa/builder.dart	(working copy)
@@ -867,7 +867,7 @@
   HGraph graph;
   LocalsHandler localsHandler;
   HInstruction rethrowableException;
-  Map<Element, HParameterValue> parameters;
+  Map<Element, HInstruction> parameters;
   final RuntimeTypeInformation rti;
 
   Map<TargetElement, JumpHandler> jumpTargets;
@@ -907,7 +907,7 @@
       stack = new List<HInstruction>(),
       activationVariables = new Map<Element, HLocalValue>(),
       jumpTargets = new Map<TargetElement, JumpHandler>(),
-      parameters = new Map<Element, HParameterValue>(),
+      parameters = new Map<Element, HInstruction>(),
       sourceElementStack = <Element>[work.element],
       inliningStack = <InliningState>[],
       rti = builder.compiler.codegenWorld.rti,
@@ -1153,6 +1153,10 @@
       FunctionSignature params = constructor.computeSignature(compiler);
       params.orderedForEachParameter((Element parameter) {
         HInstruction argument = compiledArguments[index++];
+        // Because we are inlining the initializer, we must update
+        // what was given as parameter. This will be used in case
+        // there is a parameter check expression in the initializer.
+        parameters[parameter] = argument;
         localsHandler.updateLocal(parameter, argument);
         // Don't forget to update the field, if the parameter is of the
         // form [:this.x:].
@@ -1166,7 +1170,20 @@
       TreeElements oldElements = elements;
       elements =
           compiler.enqueuer.resolution.getCachedElements(constructor);
+
+      ClosureClassMap oldClosureData = localsHandler.closureData;
+      localsHandler.closureData =
+          compiler.closureToClassMapper.computeClosureToClassMapping(
+              constructor, constructor.parseNode(compiler), elements);
+
+      params.orderedForEachParameter((Element parameterElement) {
+        if (elements.isParameterChecked(parameterElement)) {
+          addParameterCheckInstruction(parameterElement);
+        }
+      });
+
       buildInitializers(constructor, constructors, fieldValues);
+      localsHandler.closureData = oldClosureData;
       elements = oldElements;
     });
   }
@@ -1356,6 +1373,22 @@
       functionSignature.orderedForEachParameter((parameter) {
         bodyCallInputs.add(localsHandler.readLocal(parameter));
       });
+
+      // If parameters are checked, we pass what we already computed

[floitsch, 2012/10/29 21:45:45]

If parameters are checked, we pass the already computed boolean to the
constructor body.

[ngeoffray, 2012/10/30 09:34:46]

Done.

+      // for checking it to the constructor body.
+      TreeElements elements =
+          compiler.enqueuer.resolution.getCachedElements(constructor);
+      Node node = constructor.parseNode(compiler);
+      ClosureClassMap parameterClosureData =
+          compiler.closureToClassMapper.getMappingForNestedFunction(node);
+      functionSignature.orderedForEachParameter((parameter) {
+        if (elements.isParameterChecked(parameter)) {
+          Element fieldCheck =
+              parameterClosureData.parametersWithSentinel[parameter];
+          bodyCallInputs.add(localsHandler.readLocal(fieldCheck));
+        }
+      });
+
       // TODO(ahe): The constructor name is statically resolved. See
       // SsaCodeGenerator.visitInvokeDynamicMethod. Is there a cleaner
       // way to do this?
@@ -1376,48 +1409,56 @@
   }
 
   void addParameterCheckInstruction(Element element) {
-    // This is the code we emit for a parameter that is being checked
-    // on whether it was given at value at the call site:
-    //
-    // foo([a = 42) {
-    //   if (?a) print('parameter passed $a');
-    // }
-    //
-    // foo([a = 42]) {
-    //   var t1 = a === sentinel;
-    //   if (t1) a = 42;
-    //   if (!t1) print('parameter passed ' + a);
-    // }
+    HInstruction check;

[floitsch, 2012/10/29 21:45:45]

Keep the declaration at the use-point. No need to move it outside the if's. It
actually makes the method one line longer...

[ngeoffray, 2012/10/30 09:34:46]

The variable is used outside the if/else (line 1462), so I'm keeping the
declaration here.

+    Element checkResultElement =
+        localsHandler.closureData.parametersWithSentinel[element];
+    if (currentElement.isGenerativeConstructorBody()) {
+      // A generative constructor body gets the information that a
+      // parameter was passed in extra parameters.

[floitsch, 2012/10/29 21:45:45]

A generative constructor body receives extra parameters that indicate if a
parameter was passed to the factory.

[ngeoffray, 2012/10/30 09:34:46]

Done.

+      check = new HParameterValue(checkResultElement);
+      add(check);
+    } else {

[floitsch, 2012/10/29 21:45:45]

You could avoid the else (and the indentation) by just returning in the special
case. But I'm fine the way it is.

[ngeoffray, 2012/10/30 09:34:46]

No, there's a shared updateLocal call line 1462.

+      // This is the code we emit for a parameter that is being checked
+      // on whether it was given at value at the call site:
+      //
+      // foo([a = 42) {
+      //   if (?a) print('parameter passed $a');
+      // }
+      //
+      // foo([a = 42]) {
+      //   var t1 = a === sentinel;
+      //   if (t1) a = 42;
+      //   if (!t1) print('parameter passed ' + a);
+      // }
 
-    // Fetch the original default value of [element];
-    ConstantHandler handler = compiler.constantHandler;
-    Constant constant = handler.compileVariable(element);
-    HConstant defaultValue = constant == null
-        ? graph.addConstantNull(constantSystem)
-        : graph.addConstant(constant);
+      // Fetch the original default value of [element];
+      ConstantHandler handler = compiler.constantHandler;
+      Constant constant = handler.compileVariable(element);
+      HConstant defaultValue = constant == null
+          ? graph.addConstantNull(constantSystem)
+          : graph.addConstant(constant);
 
-    // Emit the equality check with the sentinel.
-    HConstant sentinel = graph.addConstant(SentinelConstant.SENTINEL);
-    Element equalsHelper = interceptors.getTripleEqualsInterceptor();
-    HInstruction target = new HStatic(equalsHelper);
-    add(target);
-    HInstruction operand = parameters[element];
-    HInstruction check = new HIdentity(target, sentinel, operand);
-    add(check);
+      // Emit the equality check with the sentinel.
+      HConstant sentinel = graph.addConstant(SentinelConstant.SENTINEL);
+      Element equalsHelper = interceptors.getTripleEqualsInterceptor();
+      HInstruction target = new HStatic(equalsHelper);
+      add(target);
+      HInstruction operand = parameters[element];
+      check = new HIdentity(target, sentinel, operand);
+      add(check);
 
-    // If the check succeeds, we must update the parameter with the
-    // default value.
-    handleIf(element.parseNode(compiler),
-             () => stack.add(check),
-             () => localsHandler.updateLocal(element, defaultValue),
-             null);
+      // If the check succeeds, we must update the parameter with the
+      // default value.
+      handleIf(element.parseNode(compiler),
+               () => stack.add(check),
+               () => localsHandler.updateLocal(element, defaultValue),
+               null);
 
-    // Create the instruction that parameter checks will use.
-    check = new HNot(check);
-    add(check);
+      // Create the instruction that parameter checks will use.
+      check = new HNot(check);
+      add(check);
+    }
 
-    ClosureClassMap closureData = localsHandler.closureData;
-    Element checkResultElement = closureData.parametersWithSentinel[element];
     localsHandler.updateLocal(checkResultElement, check);
   }