Correct change tracking and enable verification in corresponding unit…

… tests for more passes. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=153867487
google · Apr 25, 2017 · 61887b8 · 61887b8
1 parent fb1e615
commit 61887b8
Show file tree

Hide file tree

Showing 16 changed files with 30 additions and 51 deletions.
diff --git a/src/com/google/javascript/jscomp/Es6ToEs3ClassSideInheritance.java b/src/com/google/javascript/jscomp/Es6ToEs3ClassSideInheritance.java
@@ -235,7 +235,7 @@ private void copyStaticMembers(
       exprResult.useSourceInfoIfMissingFromForTree(sourceInfoNode);
       Node inheritsExpressionResult = inheritsCall.getParent();
       inheritsExpressionResult.getParent().addChildAfter(exprResult, inheritsExpressionResult);
-      compiler.reportCodeChange();
+      compiler.reportChangeToEnclosingScope(inheritsExpressionResult);
 
       // Add the static member to the subclass so that subclasses also copy this member.
       subClass.staticMembers.add(assign);

diff --git a/src/com/google/javascript/jscomp/InlineSimpleMethods.java b/src/com/google/javascript/jscomp/InlineSimpleMethods.java
@@ -110,7 +110,7 @@ void visit(NodeTraversal t, Node callNode, Node parent, String callName) {
             if (logger.isLoggable(Level.FINE)) {
               logger.fine("Inlining empty method: " + callName);
             }
-            inlineEmptyMethod(parent, callNode);
+            inlineEmptyMethod(t, parent, callNode);
           }
         }
       } else {
@@ -254,16 +254,17 @@ private void inlineConstReturn(Node parent, Node call,
   /**
    * Remove the provided object and its method call.
    */
-  private void inlineEmptyMethod(Node parent, Node call) {
+  private void inlineEmptyMethod(NodeTraversal t, Node parent, Node call) {
     // If the return value of the method call is read,
     // replace it with "void 0". Otherwise, remove the call entirely.
+
     if (NodeUtil.isExprCall(parent)) {
       parent.replaceWith(IR.empty());
     } else {
       Node srcLocation = call;
       parent.replaceChild(call, NodeUtil.newUndefinedNode(srcLocation));
     }
-    compiler.reportChangeToEnclosingScope(parent);
+    t.reportCodeChange();
   }
 
   /**

diff --git a/src/com/google/javascript/jscomp/InstrumentFunctions.java b/src/com/google/javascript/jscomp/InstrumentFunctions.java
@@ -119,13 +119,13 @@ public void process(Node externs, Node root) {
 
       Node addingRoot = compiler.getNodeForCodeInsertion(null);
       addingRoot.addChildToFront(expr.useSourceInfoIfMissingFromForTree(addingRoot));
-      compiler.reportCodeChange();
+      compiler.reportChangeToEnclosingScope(addingRoot);
     }
 
     if (initCode != null) {
       Node addingRoot = compiler.getNodeForCodeInsertion(null);
       addingRoot.addChildrenToFront(initCode);
-      compiler.reportCodeChange();
+      compiler.reportChangeToEnclosingScope(addingRoot);
     }
   }
 
@@ -207,7 +207,7 @@ void process(Node function) {
         Node call = newReportFunctionExitNode(function, null);
         Node expr = IR.exprResult(call).useSourceInfoIfMissingFromForTree(function);
         body.addChildToBack(expr);
-        compiler.reportCodeChange();
+        compiler.reportChangeToEnclosingScope(body);
       }
     }
 
@@ -320,7 +320,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
           }
           addingRoot.addChildBefore(expr, beforeChild);
         }
-        t.reportCodeChange();
+        compiler.reportChangeToEnclosingScope(addingRoot);
       }
     }
   }

diff --git a/src/com/google/javascript/jscomp/J2clConstantHoisterPass.java b/src/com/google/javascript/jscomp/J2clConstantHoisterPass.java
@@ -117,9 +117,11 @@ private void hoistConstantLikeField(Node clinitAssignment, Node declarationAssig
     Node declarationInClass = declarationAssignment.getFirstChild();
     Node declarationAssignedValue = declarationInClass.getFirstChild();
 
+    Node clinitChangeScope = NodeUtil.getEnclosingChangeScopeRoot(clinitAssignment);
     // Remove the clinit initialization
     NodeUtil.removeChild(clinitAssignment.getParent(), clinitAssignment);
 
+
     // Replace the assignment in declaration with the value from clinit
     clinitAssignedValue.detach();
     declarationInClass.replaceChild(declarationAssignedValue, clinitAssignedValue);
@@ -128,7 +130,7 @@ private void hoistConstantLikeField(Node clinitAssignment, Node declarationAssig
     // Sanity check
     checkState(NodeUtil.isLiteralValue(declarationAssignedValue, false /* includeFunctions */));
 
-    compiler.reportChangeToEnclosingScope(clinitAssignment);
+    compiler.reportChangeToChangeScope(clinitChangeScope);
     compiler.reportChangeToEnclosingScope(declarationAssignment);
   }
 

diff --git a/src/com/google/javascript/jscomp/NameAnonymousFunctionsMapped.java b/src/com/google/javascript/jscomp/NameAnonymousFunctionsMapped.java
@@ -76,9 +76,6 @@ public void process(Node externs, Node root) {
     NodeTraversal.traverseEs6(compiler, root, namingCallback);
     logger.fine("Named " + namedCount + " anon functions using " +
         bytesUsed + " bytes");
-    if (namedCount > 0) {
-      compiler.reportCodeChange();
-    }
   }
 
   /**
@@ -110,6 +107,7 @@ public final void setFunctionName(String name, Node fnNode) {
       fnNameNode.setString(newName);
       namedCount++;
       bytesUsed += newName.length();
+      compiler.reportChangeToEnclosingScope(fnNameNode);
     }
 
     String getAlternateName(String name) {

diff --git a/src/com/google/javascript/jscomp/OptimizeReturns.java b/src/com/google/javascript/jscomp/OptimizeReturns.java
@@ -21,7 +21,6 @@
 import com.google.javascript.jscomp.DefinitionsRemover.Definition;
 import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.Node;
-
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -128,8 +127,9 @@ private static boolean callResultsMaybeUsed(
   private void rewriteReturns(
       final DefinitionUseSiteFinder defFinder, Node fnNode) {
     Preconditions.checkState(fnNode.isFunction());
+    final Node body = fnNode.getLastChild();
     NodeUtil.visitPostOrder(
-      fnNode.getLastChild(),
+      body,
       new NodeUtil.Visitor() {
         @Override
         public void visit(Node node) {
@@ -144,7 +144,7 @@ public void visit(Node node) {
               node.getParent().addChildBefore(
                 IR.exprResult(result).srcref(result), node);
             }
-            compiler.reportCodeChange();
+            compiler.reportChangeToEnclosingScope(body);
           }
         }
       },

diff --git a/src/com/google/javascript/jscomp/ProcessDefines.java b/src/com/google/javascript/jscomp/ProcessDefines.java
@@ -117,28 +117,27 @@ public void process(Node externs, Node root) {
   }
 
   private void overrideDefines(Map<String, DefineInfo> allDefines) {
-    boolean changed = false;
     for (Map.Entry<String, DefineInfo> def : allDefines.entrySet()) {
       String defineName = def.getKey();
       DefineInfo info = def.getValue();
       Node inputValue = dominantReplacements.get(defineName);
       Node finalValue = inputValue != null ?
           inputValue : info.getLastValue();
       if (finalValue != info.initialValue) {
-        info.initialValueParent.replaceChild(
-            info.initialValue, finalValue.cloneTree());
         compiler.addToDebugLog("Overriding @define variable " + defineName);
-        changed =
-            changed
-                || finalValue.getToken() != info.initialValue.getToken()
-                || !finalValue.isEquivalentTo(info.initialValue);
+        boolean changed =
+            finalValue.getToken() != info.initialValue.getToken()
+            || !finalValue.isEquivalentTo(info.initialValue);
+        if (changed) {
+          info.initialValueParent.replaceChild(
+              info.initialValue, finalValue.cloneTree());
+          if (changed) {
+            compiler.reportChangeToEnclosingScope(info.initialValueParent);
+          }
+        }
       }
     }
 
-    if (changed) {
-      compiler.reportCodeChange();
-    }
-
     Set<String> unusedReplacements = Sets.difference(
         dominantReplacements.keySet(), Sets.union(KNOWN_DEFINES, allDefines.keySet()));
 

diff --git a/src/com/google/javascript/jscomp/RemoveSuperMethodsPass.java b/src/com/google/javascript/jscomp/RemoveSuperMethodsPass.java
@@ -49,8 +49,10 @@ public void process(Node externs, Node root) {
     NodeTraversal.traverseEs6(compiler, root, new RemoveSuperMethodsCallback());
     NodeTraversal.traverseEs6(compiler, root, new FilterDuplicateMethods());
     for (Map.Entry<String, Node> entry : removeCandidates.entrySet()) {
-      entry.getValue().getGrandparent().detach();
-      compiler.reportCodeChange();
+      Node removalTarget = entry.getValue().getGrandparent();
+      Node removalParent = removalTarget.getParent();
+      removalTarget.detach();
+      compiler.reportChangeToEnclosingScope(removalParent);
     }
   }
 

diff --git a/test/com/google/javascript/jscomp/Es6ToEs3ClassSideInheritanceTest.java b/test/com/google/javascript/jscomp/Es6ToEs3ClassSideInheritanceTest.java
@@ -28,7 +28,6 @@ public void setUp() {
     setAcceptedLanguage(LanguageMode.ECMASCRIPT_2015);
     setLanguageOut(LanguageMode.ECMASCRIPT3);
     allowExternsChanges(true);
-    validateAstChangeMarking(false);
   }
 
   @Override

diff --git a/test/com/google/javascript/jscomp/InlineSimpleMethodsTest.java b/test/com/google/javascript/jscomp/InlineSimpleMethodsTest.java
@@ -22,12 +22,6 @@ public InlineSimpleMethodsTest() {
     super("", false);
   }
 
-  @Override
-  protected void setUp() throws Exception {
-    super.setUp();
-    validateAstChangeMarking(false);
-  }
-
   @Override
   protected CompilerPass getProcessor(Compiler compiler) {
     return new InlineSimpleMethods(compiler);

diff --git a/test/com/google/javascript/jscomp/InstrumentFunctionsTest.java b/test/com/google/javascript/jscomp/InstrumentFunctionsTest.java
@@ -36,7 +36,6 @@ public InstrumentFunctionsTest() {
   @Override
   protected void setUp() {
     this.instrumentationPb = null;
-    validateAstChangeMarking(false);
   }
 
   @Override

diff --git a/test/com/google/javascript/jscomp/J2clConstantHoisterPassTest.java b/test/com/google/javascript/jscomp/J2clConstantHoisterPassTest.java
@@ -17,12 +17,6 @@
 
 public class J2clConstantHoisterPassTest extends CompilerTestCase {
 
-  @Override
-  protected void setUp() throws Exception {
-    super.setUp();
-    validateAstChangeMarking(false);
-  }
-
   @Override
   protected CompilerPass getProcessor(final Compiler compiler) {
     return new J2clConstantHoisterPass(compiler);

diff --git a/test/com/google/javascript/jscomp/NameAnonymousFunctionsMappedTest.java b/test/com/google/javascript/jscomp/NameAnonymousFunctionsMappedTest.java
@@ -43,7 +43,6 @@ protected int getNumRepetitions() {
   protected void setUp() throws Exception {
     super.setUp();
     previous = null;
-    validateAstChangeMarking(false);
   }
 
   @Override

diff --git a/test/com/google/javascript/jscomp/OptimizeReturnsTest.java b/test/com/google/javascript/jscomp/OptimizeReturnsTest.java
@@ -44,7 +44,6 @@ protected int getNumRepetitions() {
   protected void setUp() throws Exception {
     super.setUp();
     disableTypeCheck();
-    validateAstChangeMarking(false);
   }
 
   /**

diff --git a/test/com/google/javascript/jscomp/ProcessDefinesTest.java b/test/com/google/javascript/jscomp/ProcessDefinesTest.java
@@ -43,7 +43,6 @@ public ProcessDefinesTest() {
   @Override
   protected void setUp() throws Exception {
     super.setUp();
-    validateAstChangeMarking(false);
     overrides.clear();
   }
 

diff --git a/test/com/google/javascript/jscomp/RemoveSuperMethodsPassTest.java b/test/com/google/javascript/jscomp/RemoveSuperMethodsPassTest.java
@@ -48,12 +48,6 @@ public RemoveSuperMethodsPassTest() {
     enableTypeCheck();
   }
 
-  @Override
-  protected void setUp() throws Exception {
-    super.setUp();
-    validateAstChangeMarking(false);
-  }
-
   @Override
   public CompilerPass getProcessor(final Compiler compiler) {
     return new RemoveSuperMethodsPass(compiler);