diff --git a/src/com/google/javascript/jscomp/MarkNoSideEffectCalls.java b/src/com/google/javascript/jscomp/MarkNoSideEffectCalls.java index b70683e24a7..44a812be833 100644 --- a/src/com/google/javascript/jscomp/MarkNoSideEffectCalls.java +++ b/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); } } } diff --git a/src/com/google/javascript/jscomp/PureFunctionIdentifier.java b/src/com/google/javascript/jscomp/PureFunctionIdentifier.java index 71fd273358c..ac563f96d4a 100644 --- a/src/com/google/javascript/jscomp/PureFunctionIdentifier.java +++ b/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); + } } } diff --git a/src/com/google/javascript/rhino/Node.java b/src/com/google/javascript/rhino/Node.java index 3cad2393c67..378e3d68dff 100644 --- a/src/com/google/javascript/rhino/Node.java +++ b/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; } } diff --git a/test/com/google/javascript/jscomp/CompilerTestCase.java b/test/com/google/javascript/jscomp/CompilerTestCase.java index eedcbdcd2b3..2fadc8b0271 100644 --- a/test/com/google/javascript/jscomp/CompilerTestCase.java +++ b/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) {