Correctly mark node as changed when modified by PureFunctionIdentifier.

This is necessary since PureFunctionIdentifier runs as part of optimization loop and without nodes marked as changed, peephole passes in the optimization loop ignores them. That results in missed oppurtunities especially for J2CL.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=142511104

src/com/google/javascript/jscomp/MarkNoSideEffectCalls.java

@@ -182,6 +182,7 @@ public void visit(NodeTraversal traversal, Node node, Node parent) {
 
       if (maybeFunction) {
         node.setSideEffectFlags(Node.NO_SIDE_EFFECTS);
+        compiler.reportChangeToEnclosingScope(node);
       }
     }
   }

src/com/google/javascript/jscomp/PureFunctionIdentifier.java

@@ -403,7 +403,11 @@ private void markPureFunctionCalls() {
         }
       }
 
-      callNode.setSideEffectFlags(flags.valueOf());
+      int newSideEffectFlags = flags.valueOf();
+      if (callNode.getSideEffectFlags() != newSideEffectFlags) {
+        callNode.setSideEffectFlags(newSideEffectFlags);
+        compiler.reportChangeToEnclosingScope(callNode);
+      }
     }
   }
 

src/com/google/javascript/rhino/Node.java

@@ -284,8 +284,8 @@ public void setDouble(double d) {
 
     @Override
     boolean isEquivalentTo(
-        Node node, boolean compareType, boolean recur, boolean jsDoc) {
-      boolean equiv = super.isEquivalentTo(node, compareType, recur, jsDoc);
+        Node node, boolean compareType, boolean recur, boolean jsDoc, boolean sideEffect) {
+      boolean equiv = super.isEquivalentTo(node, compareType, recur, jsDoc, sideEffect);
       if (equiv) {
         double thisValue = getDouble();
         double thatValue = ((NumberNode) node).getDouble();
@@ -348,8 +348,8 @@ public void setString(String str) {
 
     @Override
     boolean isEquivalentTo(
-        Node node, boolean compareType, boolean recur, boolean jsDoc) {
-      return (super.isEquivalentTo(node, compareType, recur, jsDoc)
+        Node node, boolean compareType, boolean recur, boolean jsDoc, boolean sideEffect) {
+      return (super.isEquivalentTo(node, compareType, recur, jsDoc, sideEffect)
           && this.str.equals(((StringNode) node).str));
     }
 
@@ -1779,32 +1779,48 @@ private NodeMismatch checkTreeEqualsImpl(Node actual, boolean jsDoc) {
   }
 
   /** Returns true if this node is equivalent semantically to another */
-  public boolean isEquivalentTo(Node node) {
-    return isEquivalentTo(node, false, true, false);
+  public final boolean isEquivalentTo(Node node) {
+    return isEquivalentTo(node, false, true, false, false);
+  }
+
+  /** Returns true if this node is equivalent semantically to another including side efffects. */
+  public final boolean isEquivalentWithSideEffectsTo(Node node) {
+    return isEquivalentTo(node, false, true, false, true);
   }
 
   /** Checks equivalence without going into child nodes */
-  public boolean isEquivalentToShallow(Node node) {
-    return isEquivalentTo(node, false, false, false);
+  public final boolean isEquivalentToShallow(Node node) {
+    return isEquivalentTo(node, false, false, false, false);
   }
 
   /**
-   * Returns true if this node is equivalent semantically to another and
-   * the types are equivalent.
+   * Returns true if this node is equivalent semantically to another and the types are equivalent.
    */
-  public boolean isEquivalentToTyped(Node node) {
-    return isEquivalentTo(node, true, true, true);
+  public final boolean isEquivalentToTyped(Node node) {
+    return isEquivalentTo(node, true, true, true, false);
   }
 
   /**
    * @param compareType Whether to compare the JSTypes of the nodes.
-   * @param recurse Whether to compare the children of the current node, if
-   *    not only the the count of the children are compared.
+   * @param recurse Whether to compare the children of the current node, if not only the the count
+   *     of the children are compared.
    * @param jsDoc Whether to check that the JsDoc of the nodes are equivalent.
    * @return Whether this node is equivalent semantically to the provided node.
    */
+  final boolean isEquivalentTo(Node node, boolean compareType, boolean recurse, boolean jsDoc) {
+    return isEquivalentTo(node, compareType, recurse, jsDoc, false);
+  }
+
+  /**
+   * @param compareType Whether to compare the JSTypes of the nodes.
+   * @param recurse Whether to compare the children of the current node, if not only the the count
+   *     of the children are compared.
+   * @param jsDoc Whether to check that the JsDoc of the nodes are equivalent.
+   * @param sideEffect Whether to check that the side-effect flags of the nodes are equivalent.
+   * @return Whether this node is equivalent semantically to the provided node.
+   */
   boolean isEquivalentTo(
-      Node node, boolean compareType, boolean recurse, boolean jsDoc) {
+      Node node, boolean compareType, boolean recurse, boolean jsDoc, boolean sideEffect) {
     if (token != node.token
         || getChildCount() != node.getChildCount()
         || this.getClass() != node.getClass()) {
@@ -1857,11 +1873,17 @@ boolean isEquivalentTo(
       }
     }
 
+    if (sideEffect) {
+      if (this.getSideEffectFlags() != node.getSideEffectFlags()) {
+        return false;
+      }
+    }
+
     if (recurse) {
       for (Node n = first, n2 = node.first;
            n != null;
            n = n.next, n2 = n2.next) {
-        if (!n.isEquivalentTo(n2, compareType, recurse, jsDoc)) {
+        if (!n.isEquivalentTo(n2, compareType, recurse, jsDoc, sideEffect)) {
           return false;
         }
       }

test/com/google/javascript/jscomp/CompilerTestCase.java

@@ -1319,9 +1319,11 @@ private void test(
         }
 
         if (computeSideEffects && i == 0) {
+          recentChange.reset();
           PureFunctionIdentifier.Driver mark =
               new PureFunctionIdentifier.Driver(compiler, null);
           mark.process(externsRoot, mainRoot);
+          hasCodeChanged = hasCodeChanged || recentChange.hasCodeChanged();
         }
 
         if (markNoSideEffects && i == 0) {
@@ -1416,8 +1418,8 @@ private void test(
         normalizeActualCode(compiler, externsRootClone, mainRootClone);
       }
 
-      boolean codeChange = !mainRootClone.isEquivalentTo(mainRoot);
-      boolean externsChange = !externsRootClone.isEquivalentTo(externsRoot);
+      boolean codeChange = !mainRootClone.isEquivalentWithSideEffectsTo(mainRoot);
+      boolean externsChange = !externsRootClone.isEquivalentWithSideEffectsTo(externsRoot);
 
       // Generally, externs should not be changed by the compiler passes.
       if (externsChange && !allowExternsChanges) {