Rename ClosureVariable, use separate IR forms for declaration and assignment.

pkg/compiler/lib/src/cps_ir/type_propagation.dart

Index: pkg/compiler/lib/src/cps_ir/type_propagation.dart
diff --git a/pkg/compiler/lib/src/cps_ir/type_propagation.dart b/pkg/compiler/lib/src/cps_ir/type_propagation.dart
index c6dd7abfc987bb55832e039392153160ae560a4d..4c8f97cd37d4c7c2783314e24674f9965c592907 100644
--- a/pkg/compiler/lib/src/cps_ir/type_propagation.dart
+++ b/pkg/compiler/lib/src/cps_ir/type_propagation.dart
@@ -415,7 +415,8 @@ class _TypePropagationVisitor<T> extends Visitor {
 
   void visitNode(Node node) {
     internalError(NO_LOCATION_SPANNABLE,
-        "_TypePropagationVisitor is stale, add missing visit overrides");
+        "_TypePropagationVisitor is stale,"
+        " add missing visit overrides (${node.runtimeType})");

[karlklose, 2015/02/03 09:56:16]

Please do not use '.runtimeType. in production code, it requires runtime type
support.  Normally 'toString' should include sufficient information.

[Kevin Millikin (Google), 2015/02/03 14:11:49]

Done.

   }
 
   void visitRunnableBody(RunnableBody node) {
@@ -445,6 +446,11 @@ class _TypePropagationVisitor<T> extends Visitor {
     setReachable(node.body);
   }
 
+  void visitLetMutable(LetMutable node) {
+    setValue(node.variable, getValue(node.value.definition));
+    setReachable(node.body);
+  }
+
   void visitInvokeStatic(InvokeStatic node) {
     Continuation cont = node.continuation.definition;
     setReachable(cont);
@@ -667,7 +673,8 @@ class _TypePropagationVisitor<T> extends Visitor {
     }
   }
 
-  void visitSetClosureVariable(SetClosureVariable node) {
+  void visitSetMutableVariable(SetMutableVariable node) {
+    setValue(node.variable.definition, getValue(node.value.definition));
     setReachable(node.body);
   }
 
@@ -710,11 +717,28 @@ class _TypePropagationVisitor<T> extends Visitor {
     setValue(node, constantValue(constant, typeSystem.functionType));
   }
 
-  void visitGetClosureVariable(GetClosureVariable node) {
-    setValue(node, nonConst());
+  void visitGetMutableVariable(GetMutableVariable node) {
+    setValue(node, getValue(node.variable.definition));

[asgerf, 2015/02/03 10:27:31]

When targeting Dart I don't think we can track mutable variables like this,
because they can be reassigned inside a nested function.

We could either default to nonConst() while targeting Dart, or detect mutable
variables that are assigned inside nested functions.

For JS it should be fine, of course, since mutable variables are local to one
function activation.

[asgerf, 2015/02/03 10:30:26]

Actually on second thought, I didn't realize this was a flow-insensitive
analysis. It should be fine.

[Kevin Millikin (Google), 2015/02/03 14:11:49]

Yes, but I'll add a comment explaining why this is OK.

   }
 
-  void visitClosureVariable(ClosureVariable node) {
+  void visitMutableVariable(MutableVariable node) {
+    // [MutableVariable]s are bound either as parameters to
+    // [FunctionDefinition]s, by [LetMutable], or by [DeclareFunction].
+    if (node.parent is FunctionDefinition) {
+      // Just like immutable parameters, the values of mutable parameters are
+      // never constant.
+      // TODO(karlklose): remove reference to the element model.
+      Entity source = node.hint;
+      T type = (source is ParameterElement)
+          ? typeSystem.getParameterType(source)
+          : typeSystem.dynamicType;
+      setValue(node, nonConst(type));
+    } else if (node.parent is LetMutable || node.parent is DeclareFunction) {
+      // Mutable values bound by LetMutable or DeclareFunction could have
+      // known values.
+    } else {
+      internalError(node.hint, "Unexpected parent of MutableVariable");
+    }
   }
 
   void visitParameter(Parameter node) {
@@ -723,24 +747,18 @@ class _TypePropagationVisitor<T> extends Visitor {
     T type = (source is ParameterElement) ? typeSystem.getParameterType(source)
         : typeSystem.dynamicType;

[asgerf, 2015/02/03 10:27:31]

Off-topic: Is type propagation relying on type annotations?

[Kevin Millikin (Google), 2015/02/03 14:11:49]

At the moment it doesn't.

     if (node.parent is FunctionDefinition) {
-      // Functions may escape and thus their parameters must be initialized to
-      // NonConst.
+      // Functions may escape and thus their parameters must be non-constant.
       setValue(node, nonConst(type));
     } else if (node.parent is Continuation) {
-      // Continuations on the other hand are local, and parameters are
-      // initialized to Unknown.
-      setValue(node, unknown());
+      // Continuations on the other hand are local, and parameters can have
+      // some other abstract value than non-constant.
     } else {
       internalError(node.hint, "Unexpected parent of Parameter");
     }
   }
 
   void visitContinuation(Continuation node) {
-    node.parameters.forEach((Parameter p) {
-      // TODO(karlklose): join parameter types from use sites.
-      setValue(p, unknown());
-      defWorkset.add(p);
-    });
+    node.parameters.forEach(visit);
 
     if (node.body != null) {
       setReachable(node.body);