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 e73dd846c674e07c154c69781fc54ade0d53c937..e0752b60e0615c0f3edf8499428ba23dbf37ec23 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,8 @@ class RefinedUseIterable extends IterableBase<Reference<Primitive>> {
 abstract class Primitive extends Variable<Primitive> {
   Primitive() : super(null);
 
+  int get effectFlags => Effects.none;

[Siggi Cherem (dart-lang), 2016/02/17 20:53:23]

+ dartdoc

Also, low-priority but, what do you think about simply calling this `effects`?

[asgerf, 2016/02/29 12:48:17]

Done.

+
   /// True if this primitive has a value that can be used by other expressions.
   bool get hasValue;
 
@@ -345,10 +350,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 effectFlags = Effects.all;
   bool isSafeForElimination = false;
   bool isSafeForReordering = false;
 }
@@ -1083,6 +1086,8 @@ class ApplyBuiltinMethod extends Primitive {
     receiver.parent = this;
     _setParentsOnList(arguments, this);
   }
+
+  int get effectFlags => getEffectsOfBuiltinMethod(method);
 }
 
 /// Throw a value.
@@ -1278,6 +1283,8 @@ class SetField extends Primitive {
     object.parent = this;
     value.parent = this;
   }
+
+  int get effectFlags => Effects.changesInstanceField;
 }
 
 /// Directly reads from a field on a given object.
@@ -1286,6 +1293,7 @@ class SetField extends Primitive {
 class GetField extends Primitive {
   final Reference<Primitive> object;
   FieldElement field;
+  bool isFinal = false;

[Siggi Cherem (dart-lang), 2016/02/17 20:53:23]

+dartdoc

[asgerf, 2016/02/29 12:48:17]

Done.

 
   /// True if the object is known not to be null.
   // TODO(asgerf): This is a placeholder until we agree on how to track
@@ -1306,11 +1314,14 @@ class GetField extends Primitive {
   void setParentPointers() {
     object.parent = this;
   }
+
+  int get effectFlags => isFinal ? 0 : Effects.dependsOnInstanceField;
 }
 
 /// Get the length of a string or native list.
 class GetLength extends Primitive {
   final Reference<Primitive> object;
+  bool isFinal = false;
 
   /// True if the object is known not to be null.
   bool objectIsNotNull = false;
@@ -1326,6 +1337,8 @@ class GetLength extends Primitive {
   void setParentPointers() {
     object.parent = this;
   }
+
+  int get effectFlags => isFinal ? 0 : Effects.dependsOnIndexableLength;
 }
 
 /// Read an entry from an indexable object.
@@ -1353,11 +1366,16 @@ class GetIndex extends Primitive {
     object.parent = this;
     index.parent = this;
   }
+
+  int get effectFlags => 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 {
@@ -1381,6 +1399,8 @@ class SetIndex extends Primitive {
     index.parent = this;
     value.parent = this;
   }
+
+  int get effectFlags => Effects.changesIndexableContent;
 }
 
 /// Reads the value of a static field or tears off a static method.
@@ -1392,6 +1412,7 @@ class GetStatic extends Primitive {
   /// Can be [FieldElement] or [FunctionElement].
   final Element element;
   final SourceInformation sourceInformation;
+  bool isFinal = false;
 
   /// If reading a lazily initialized field, [witness] must refer to a node
   /// that initializes the field or always occurs after the field initializer.
@@ -1410,15 +1431,15 @@ class GetStatic extends Primitive {
 
   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 effectFlags => isFinal ? 0 : Effects.dependsOnStaticField;
 }
 
 /// Sets the value of a static field.
@@ -1439,6 +1460,8 @@ class SetStatic extends Primitive {
   void setParentPointers() {
     value.parent = this;
   }
+
+  int get effectFlags => Effects.changesStaticField;
 }
 
 /// Reads the value of a lazily initialized static field.
@@ -1448,6 +1471,7 @@ class SetStatic extends Primitive {
 class GetLazyStatic extends UnsafePrimitive {
   final FieldElement element;
   final SourceInformation sourceInformation;
+  bool isFinal = false;
 
   GetLazyStatic(this.element, [this.sourceInformation]);
 
@@ -1456,6 +1480,9 @@ class GetLazyStatic extends UnsafePrimitive {
   bool get hasValue => true;
 
   void setParentPointers() {}
+
+  // TODO(asgerf): Track side effects of lazy field initializers.
+  int get effectFlags => Effects.all;
 }
 
 /// Creates an object for holding boxed variables captured by a closure.
@@ -1575,7 +1602,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) {
+    effectFlags = Effects.from(nativeBehavior.sideEffects);
+  }
 
   accept(Visitor visitor) => visitor.visitForeignCode(this);
 
@@ -2564,6 +2593,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();
 
@@ -2633,7 +2667,8 @@ class DefinitionCopyingVisitor extends Visitor<Definition> {
   }
 
   Definition visitGetLazyStatic(GetLazyStatic node) {
-    return new GetLazyStatic(node.element, node.sourceInformation);
+    return new GetLazyStatic(node.element, node.sourceInformation)
+        ..isFinal = node.isFinal;
   }
 
   Definition visitAwait(Await node) {
@@ -2666,7 +2701,13 @@ 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),
+          node.sourceInformation);
+    } else {
+      return new GetStatic(node.element, node.sourceInformation);
+    }
   }
 
   Definition visitInterceptor(Interceptor node) {
@@ -2683,7 +2724,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) {
@@ -2752,7 +2794,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);
     }
@@ -2760,7 +2802,7 @@ class DefinitionCopyingVisitor extends Visitor<Definition> {
 
   Definition visitNullCheck(NullCheck node) {
     return new NullCheck(getCopy(node.value), node.sourceInformation,
-        condition: node.condition == null ? null : getCopy(node.condition),
+        condition: getCopyOrNull(node.condition),
         selector: node.selector,
         useSelector: node.useSelector);
   }