Skip to content

Commit

Permalink
Adds a deleteTimeline with recording/querying/testing.
Browse files Browse the repository at this point in the history
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=158584751
  • Loading branch information
stalcup authored and Tyler Breisacher committed Jun 13, 2017
1 parent 2a6063b commit 1423f1b
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 10 deletions.
7 changes: 7 additions & 0 deletions src/com/google/javascript/jscomp/AbstractCompiler.java
Expand Up @@ -322,6 +322,13 @@ LifeCycleStage getLifeCycleStage() {
*/
abstract List<Node> getChangedScopeNodesForPass(String passName);

/**
* An accumulation of deleted scope nodes since the last time the given pass was run. A returned
* null or empty list means no scope nodes have been deleted since the last run or this is the
* first time the pass has run.
*/
abstract List<Node> getDeletedScopeNodesForPass(String passName);

/** Called to indicate that the current change stamp has been used */
abstract void incrementChangeStamp();

Expand Down
18 changes: 18 additions & 0 deletions src/com/google/javascript/jscomp/Compiler.java
Expand Up @@ -302,6 +302,7 @@ public SourceFile loadSource(String filename) {
private int changeStamp = 1;

private final Timeline<Node> changeTimeline = new Timeline<>();
private final Timeline<Node> deleteTimeline = new Timeline<>();

/**
* Creates a Compiler that reports errors and warnings to its logger.
Expand Down Expand Up @@ -2579,6 +2580,13 @@ List<Node> getChangedScopeNodesForPass(String passName) {
return changedScopeNodes;
}

@Override
List<Node> getDeletedScopeNodesForPass(String passName) {
List<Node> deletedScopeNodes = deleteTimeline.getSince(passName);
deleteTimeline.mark(passName);
return deletedScopeNodes;
}

@Override
public void incrementChangeStamp() {
changeStamp++;
Expand Down Expand Up @@ -2610,6 +2618,15 @@ private Node getChangeScopeForNode(Node n) {
}

private void recordChange(Node n) {
if (n.isDeleted()) {
// Some complicated passes (like SmartNameRemoval) might both change and delete a scope in
// the same pass, and they might even perform the change after the deletion because of
// internal queueing. Just ignore the spurious attempt to mark changed after already marking
// deleted. There's no danger of deleted nodes persisting in the AST since this is enforced
// separately in ChangeVerifier.
return;
}

n.setChangeTime(changeStamp);
// Every code change happens at a different time
changeStamp++;
Expand Down Expand Up @@ -2653,6 +2670,7 @@ public void reportFunctionDeleted(Node n) {
checkState(n.isFunction());
n.setDeleted(true);
changeTimeline.remove(n);
deleteTimeline.add(n);
}

@Override
Expand Down
95 changes: 85 additions & 10 deletions test/com/google/javascript/jscomp/CompilerTest.java
Expand Up @@ -1122,30 +1122,105 @@ public void testReportChangeWithScopeSucceeds() {
compiler.reportChangeToEnclosingScope(attachedNode);
}

/** See TimelineTest.java for the many behavior tests that don't make sense to duplicate here. */
public void testGetChangedScopeNodesForPass_doesntIncludeDeleted() {
/**
* See TimelineTest.java for the many timeline behavior tests that don't make sense to duplicate
* here.
*/
public void testGetChangesAndDeletions_baseline() {
Compiler compiler = new Compiler();

// In the initial state nothing has been marked changed or deleted.
assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner")).isNull();
assertThat(compiler.getDeletedScopeNodesForPass("FunctionInliner")).isNull();
}

public void testGetChangesAndDeletions_changeReportsVisible() {
Compiler compiler = new Compiler();
Node function1 = IR.function(IR.name("foo"), IR.paramList(), IR.block());
Node function2 = IR.function(IR.name("foo"), IR.paramList(), IR.block());
IR.root(IR.script(function1, function2));

// In the initial state everything is attached and nothing has been marked changed.
assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner")).isNull();
// Mark original baseline.
compiler.getChangedScopeNodesForPass("FunctionInliner");
compiler.getDeletedScopeNodesForPass("FunctionInliner");

// Mark both functions changed.
compiler.reportChangeToChangeScope(function1);
compiler.reportChangeToChangeScope(function2);

// Both function1 and function2 are seen as changed and nothing is seen as deleted.
assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner"))
.containsExactly(function1, function2);
assertThat(compiler.getDeletedScopeNodesForPass("FunctionInliner")).isEmpty();
}

public void testGetChangesAndDeletions_deleteOverridesChange() {
Compiler compiler = new Compiler();
Node function1 = IR.function(IR.name("foo"), IR.paramList(), IR.block());
Node function2 = IR.function(IR.name("foo"), IR.paramList(), IR.block());
IR.root(IR.script(function1, function2));

// Mark original baseline.
compiler.getChangedScopeNodesForPass("FunctionInliner");
compiler.getDeletedScopeNodesForPass("FunctionInliner");

// Mark both functions changed, then delete function2 and mark it deleted.
compiler.reportChangeToChangeScope(function1);
compiler.reportChangeToChangeScope(function2);
function2.detach();
compiler.reportFunctionDeleted(function2);

// Only function1 will be seen as changed, since deleting something removes it from the changed
// items list.
assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner"))
.containsExactly(function1);
// Now function1 will be seen as changed and function2 will be seen as deleted, since delete
// overrides change.
assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner")).containsExactly(function1);
assertThat(compiler.getDeletedScopeNodesForPass("FunctionInliner")).containsExactly(function2);
}

public void testGetChangesAndDeletions_changeDoesntOverrideDelete() {
Compiler compiler = new Compiler();
Node function1 = IR.function(IR.name("foo"), IR.paramList(), IR.block());
Node function2 = IR.function(IR.name("foo"), IR.paramList(), IR.block());
IR.root(IR.script(function1, function2));

// Mark original baseline.
compiler.getChangedScopeNodesForPass("FunctionInliner");
compiler.getDeletedScopeNodesForPass("FunctionInliner");

// Mark function1 changed and function2 deleted, then try to mark function2 changed.
compiler.reportChangeToChangeScope(function1);
function2.detach();
compiler.reportFunctionDeleted(function2);
compiler.reportChangeToChangeScope(function2);

// Now function1 will be seen as changed and function2 will be seen as deleted, since change
// does not override delete.
assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner")).containsExactly(function1);
assertThat(compiler.getDeletedScopeNodesForPass("FunctionInliner")).containsExactly(function2);
}

public void testGetChangesAndDeletions_onlySeesChangesSinceLastRequest() {
Compiler compiler = new Compiler();
Node function1 = IR.function(IR.name("foo"), IR.paramList(), IR.block());
Node function2 = IR.function(IR.name("foo"), IR.paramList(), IR.block());
IR.root(IR.script(function1, function2));

// Mark original baseline.
compiler.getChangedScopeNodesForPass("FunctionInliner");
compiler.getDeletedScopeNodesForPass("FunctionInliner");

// Mark function1 changed and function2 deleted.
compiler.reportChangeToChangeScope(function1);
function2.detach();
compiler.reportFunctionDeleted(function2);

// Verify their respective states are seen.
assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner")).containsExactly(function1);
assertThat(compiler.getDeletedScopeNodesForPass("FunctionInliner")).containsExactly(function2);

// Asking for changes again will now show nothing, since no changes have occurred between these
// consecutive "FunctionInliner" markings.
// Check states again. Should find nothing since nothing has changed since the last
// 'FunctionInliner' request.
assertThat(compiler.getChangedScopeNodesForPass("FunctionInliner")).isEmpty();
assertThat(compiler.getDeletedScopeNodesForPass("FunctionInliner")).isEmpty();
}

private static CompilerOptions createNewFlagBasedOptions() {
Expand Down

0 comments on commit 1423f1b

Please sign in to comment.