dart2js cps: Refactor tracking of side effects.

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

Index: pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart
diff --git a/pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart b/pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart
index 156226216205f4d85cdc75c815e95e09ede3e3ed..c41dd84d555b4a780bdf0681d00cf509a2eacba2 100644
--- a/pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart
+++ b/pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart
@@ -12,10 +12,13 @@ import '../elements/elements.dart';
 import '../io/source_information.dart' show SourceInformation;
 import '../types/types.dart' show TypeMask;
 import '../universe/selector.dart' show Selector;
+import '../universe/side_effects.dart';
 
 import 'builtin_operator.dart';
 export 'builtin_operator.dart';
 
+import 'effects.dart';
+
 // These imports are only used for the JavaScript specific nodes.  If we want to
 // support more than one native backend, we should probably create better
 // abstractions for native code and its type and effect system.
@@ -242,6 +245,10 @@ class RefinedUseIterable extends IterableBase<Reference<Primitive>> {
 abstract class Primitive extends Variable<Primitive> {
   Primitive() : super(null);
 
+  /// Returns a bitmask with the non-local side effects and dependencies of
+  /// this primitive, as defined by [Effects].
+  int get effects => Effects.none;
+
   /// True if this primitive has a value that can be used by other expressions.
   bool get hasValue;
 
@@ -345,10 +352,8 @@ abstract class Primitive extends Variable<Primitive> {
 
 /// A primitive that is generally not safe for elimination, but may be marked
 /// as safe by type propagation
-//
-// TODO(asgerf): Store the flag in a bitmask in [Primitive] and get rid of this
-//               class.
 abstract class UnsafePrimitive extends Primitive {
+  int effects = Effects.all;
   bool isSafeForElimination = false;
   bool isSafeForReordering = false;
 }
@@ -1143,6 +1148,8 @@ class ApplyBuiltinMethod extends Primitive {
     receiver.parent = this;
     _setParentsOnList(arguments, this);
   }
+
+  int get effects => getEffectsOfBuiltinMethod(method);
 }
 
 /// Throw a value.
@@ -1338,6 +1345,8 @@ class SetField extends Primitive {
     object.parent = this;
     value.parent = this;
   }
+
+  int get effects => Effects.changesInstanceField;
 }
 
 /// Directly reads from a field on a given object.
@@ -1347,12 +1356,15 @@ class GetField extends Primitive {
   final Reference<Primitive> object;
   FieldElement field;
 
+  /// True if the field never changes value.
+  final bool isFinal;
+
   /// True if the object is known not to be null.
   // TODO(asgerf): This is a placeholder until we agree on how to track
   //               side effects.
   bool objectIsNotNull = false;
 
-  GetField(Primitive object, this.field)
+  GetField(Primitive object, this.field, {this.isFinal: false})
       : this.object = new Reference<Primitive>(object);
 
   accept(Visitor visitor) => visitor.visitGetField(this);
@@ -1366,16 +1378,22 @@ class GetField extends Primitive {
   void setParentPointers() {
     object.parent = this;
   }
+
+  int get effects => isFinal ? 0 : Effects.dependsOnInstanceField;

[sra1, 2016/02/29 23:40:33]

isFinal ? Effects.none : Effects.dependsOnInstanceField

 }
 
 /// Get the length of a string or native list.
 class GetLength extends Primitive {
   final Reference<Primitive> object;
 
+  /// True if the length of the given object can never change.
+  bool isFinal;
+
   /// True if the object is known not to be null.
   bool objectIsNotNull = false;
 
-  GetLength(Primitive object) : this.object = new Reference<Primitive>(object);
+  GetLength(Primitive object, {this.isFinal: false})
+      : this.object = new Reference<Primitive>(object);
 
   bool get hasValue => true;
   bool get isSafeForElimination => objectIsNotNull;
@@ -1386,6 +1404,8 @@ class GetLength extends Primitive {
   void setParentPointers() {
     object.parent = this;
   }
+
+  int get effects => isFinal ? 0 : Effects.dependsOnIndexableLength;
 }
 
 /// Read an entry from an indexable object.
@@ -1413,11 +1433,16 @@ class GetIndex extends Primitive {
     object.parent = this;
     index.parent = this;
   }
+
+  int get effects => Effects.dependsOnIndexableContent;
 }
 
 /// Set an entry on a native list.
 ///
-/// [object] must be null or a native list, and [index] must be an integer.
+/// [object] must be null or a native list, and [index] must be an integer
+/// within the bounds of the indexable object.
+///
+/// [SetIndex] may not be used to alter the length of a JS array.
 ///
 /// The primitive itself has no value and may not be referenced.
 class SetIndex extends Primitive {
@@ -1441,6 +1466,8 @@ class SetIndex extends Primitive {
     index.parent = this;
     value.parent = this;
   }
+
+  int get effects => Effects.changesIndexableContent;
 }
 
 /// Reads the value of a static field or tears off a static method.
@@ -1453,32 +1480,36 @@ class GetStatic extends Primitive {
   final Element element;
   final SourceInformation sourceInformation;
 
+  /// True if the field never changes value.
+  final bool isFinal;
+
   /// If reading a lazily initialized field, [witness] must refer to a node
   /// that initializes the field or always occurs after the field initializer.
   ///
   /// The value of the witness is not used.
   Reference<Primitive> witness;
 
-  GetStatic(this.element, [this.sourceInformation]);
+  GetStatic(this.element, {this.isFinal: false, this.sourceInformation});
 
   /// Read a lazily initialized static field that is known to have been
   /// initialized by [witness] or earlier.
-  GetStatic.witnessed(this.element, Primitive witness, [this.sourceInformation])
-      : witness = witness == null ? null : new Reference<Primitive>(witness);
+  GetStatic.witnessed(this.element, Primitive witness, {this.sourceInformation})
+      : witness = witness == null ? null : new Reference<Primitive>(witness),
+        isFinal = false;
 
   accept(Visitor visitor) => visitor.visitGetStatic(this);
 
   bool get hasValue => true;
   bool get isSafeForElimination => true;
-  bool get isSafeForReordering {
-    return element is FunctionElement || element.isFinal;
-  }
+  bool get isSafeForReordering => isFinal;
 
   void setParentPointers() {
     if (witness != null) {
       witness.parent = this;
     }
   }
+
+  int get effects => isFinal ? 0 : Effects.dependsOnStaticField;
 }
 
 /// Sets the value of a static field.
@@ -1499,6 +1530,8 @@ class SetStatic extends Primitive {
   void setParentPointers() {
     value.parent = this;
   }
+
+  int get effects => Effects.changesStaticField;
 }
 
 /// Reads the value of a lazily initialized static field.
@@ -1509,13 +1542,19 @@ class GetLazyStatic extends UnsafePrimitive {
   final FieldElement element;
   final SourceInformation sourceInformation;
 
-  GetLazyStatic(this.element, [this.sourceInformation]);
+  /// True if the field never changes value.
+  final bool isFinal;
+
+  GetLazyStatic(this.element, {this.isFinal: false, this.sourceInformation});
 
   accept(Visitor visitor) => visitor.visitGetLazyStatic(this);
 
   bool get hasValue => true;
 
   void setParentPointers() {}
+
+  // TODO(asgerf): Track side effects of lazy field initializers.
+  int get effects => Effects.all;
 }
 
 /// Creates an object for holding boxed variables captured by a closure.
@@ -1635,7 +1674,9 @@ class ForeignCode extends UnsafePrimitive {
 
   ForeignCode(this.codeTemplate, this.storedType, List<Primitive> arguments,
       this.nativeBehavior, {this.dependency})
-      : this.arguments = _referenceList(arguments);
+      : this.arguments = _referenceList(arguments) {
+    effects = Effects.from(nativeBehavior.sideEffects);
+  }
 
   accept(Visitor visitor) => visitor.visitForeignCode(this);
 
@@ -2636,6 +2677,11 @@ class DefinitionCopyingVisitor extends Visitor<Definition> {
   /// Get the copy of a [Reference]'s definition from the map.
   Definition getCopy(Reference reference) => _copies[reference.definition];
 
+  /// Get the copy of a [Reference]'s definition from the map.
+  Definition getCopyOrNull(Reference reference) => reference == null
+      ? null
+      : getCopy(reference);
+
   /// Map a list of [Reference]s to the list of their definition's copies.
   List<Definition> getList(List<Reference> list) => list.map(getCopy).toList();
 
@@ -2705,7 +2751,9 @@ class DefinitionCopyingVisitor extends Visitor<Definition> {
   }
 
   Definition visitGetLazyStatic(GetLazyStatic node) {
-    return new GetLazyStatic(node.element, node.sourceInformation);
+    return new GetLazyStatic(node.element,
+        isFinal: node.isFinal,
+        sourceInformation: node.sourceInformation);
   }
 
   Definition visitAwait(Await node) {
@@ -2738,7 +2786,15 @@ class DefinitionCopyingVisitor extends Visitor<Definition> {
   }
 
   Definition visitGetStatic(GetStatic node) {
-    return new GetStatic(node.element, node.sourceInformation);
+    if (node.witness != null) {
+      return new GetStatic.witnessed(node.element,
+          getCopy(node.witness),
+          sourceInformation: node.sourceInformation);
+    } else {
+      return new GetStatic(node.element,
+          isFinal: node.isFinal,
+          sourceInformation: node.sourceInformation);
+    }
   }
 
   Definition visitInterceptor(Interceptor node) {
@@ -2755,7 +2811,8 @@ class DefinitionCopyingVisitor extends Visitor<Definition> {
   }
 
   Definition visitGetField(GetField node) {
-    return new GetField(getCopy(node.object), node.field);
+    return new GetField(getCopy(node.object), node.field,
+        isFinal: node.isFinal);
   }
 
   Definition visitCreateBox(CreateBox node) {
@@ -2802,7 +2859,7 @@ class DefinitionCopyingVisitor extends Visitor<Definition> {
   }
 
   Definition visitGetLength(GetLength node) {
-    return new GetLength(getCopy(node.object));
+    return new GetLength(getCopy(node.object), isFinal: node.isFinal);
   }
 
   Definition visitGetIndex(GetIndex node) {
@@ -2824,7 +2881,7 @@ class DefinitionCopyingVisitor extends Visitor<Definition> {
           node.sourceInformation);
     } else {
       return new BoundsCheck(getCopy(node.object), getCopy(node.index),
-          node.length == null ? null : getCopy(node.length),
+          getCopyOrNull(node.length),
           node.checks,
           node.sourceInformation);
     }
@@ -2834,7 +2891,7 @@ class DefinitionCopyingVisitor extends Visitor<Definition> {
     return new ReceiverCheck(getCopy(node.value),
         node.selector,
         node.sourceInformation,
-        condition: node.condition == null ? null : getCopy(node.condition),
+        condition: getCopyOrNull(node.condition),
         useSelector: node.useSelector,
         isNullCheck: node.isNullCheck);
   }