Skip to content

Commit d498e43

Browse files
committed
Bugfix for sanity check of changed scopes.
Clone the AST at the beginning of a loopable pass, instead of after the sanity check. This way, if there are two optimization loops, and the non-loopable passes between the loops change the AST, the 1st pass of the 2nd loop doesn't use a stale clone for its sanity check. Also, don't do the scope-change sanity check for cross-module-code-motion, it's not relevant. ------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=66241818
1 parent 20cd1d0 commit d498e43

File tree

4 files changed

+44
-4
lines changed

4 files changed

+44
-4
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ public class Compiler extends AbstractCompiler {
103103

104104
// Used in PerformanceTracker
105105
static final String PARSING_PASS_NAME = "parseInputs";
106+
static final String CROSS_MODULE_CODE_MOTION_NAME = "crossModuleCodeMotion";
106107

107108
private static final String CONFIG_RESOURCE =
108109
"com.google.javascript.jscomp.parsing.ParserConfig";

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1962,7 +1962,7 @@ protected CompilerPass create(AbstractCompiler compiler) {
19621962
* Move global symbols to a deeper common module
19631963
*/
19641964
final PassFactory crossModuleCodeMotion =
1965-
new PassFactory("crossModuleCodeMotion", false) {
1965+
new PassFactory(Compiler.CROSS_MODULE_CODE_MOTION_NAME, false) {
19661966
@Override
19671967
protected CompilerPass create(AbstractCompiler compiler) {
19681968
return new CrossModuleCodeMotion(

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

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,10 @@ void setSanityCheck(PassFactory sanityCheck) {
187187
}
188188

189189
private void setSanityCheckState() {
190-
lastAst = jsRoot.cloneTree();
191-
mtoc = NodeUtil.mapMainToClone(jsRoot, lastAst);
190+
if (inLoop) {
191+
lastAst = jsRoot.cloneTree();
192+
mtoc = NodeUtil.mapMainToClone(jsRoot, lastAst);
193+
}
192194
}
193195

194196
/**
@@ -236,7 +238,6 @@ private void maybeSanityCheck(Node externs, Node root) {
236238
sanityCheck.create(compiler).process(externs, root);
237239
if (inLoop) {
238240
NodeUtil.verifyScopeChanges(mtoc, jsRoot, true, compiler);
239-
setSanityCheckState();
240241
}
241242
}
242243
}
@@ -261,6 +262,18 @@ class NamedPass implements CompilerPass {
261262
@Override
262263
public void process(Node externs, Node root) {
263264
logger.fine(name);
265+
// Cross-module code motion is a loopable pass on its own, but does not
266+
// participate in the other optimization loops, and is not relevant to
267+
// tracking changed scopes.
268+
// Don't set inLoop, to avoid the special sanity check.
269+
if (name.equals(Compiler.CROSS_MODULE_CODE_MOTION_NAME)) {
270+
inLoop = false;
271+
}
272+
if (sanityCheck != null) {
273+
// Before running the pass, clone the AST so you can sanity-check the
274+
// changed AST against the clone after the pass finishes.
275+
setSanityCheckState();
276+
}
264277
if (tracker != null) {
265278
tracker.recordPassStart(name, factory.isOneTimePass());
266279
}

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.javascript.jscomp;
1818

1919
import com.google.common.collect.Lists;
20+
import com.google.javascript.rhino.Node;
2021

2122
import java.util.List;
2223

@@ -120,6 +121,14 @@ public void testTopScopeChange() {
120121
test("var x = 1, y = x, z = x + y;", "var z = 2;");
121122
}
122123

124+
public void testTwoOptimLoopsNoCrash() {
125+
passes = Lists.newLinkedList();
126+
addInlineVariables();
127+
addSmartNamePass();
128+
addInlineVariables();
129+
test("var x = '';", "");
130+
}
131+
123132
private void addCollapseObjectLiterals() {
124133
passes.add(
125134
new PassFactory("collapseObjectLiterals", false) {
@@ -200,4 +209,21 @@ protected CompilerPass create(AbstractCompiler compiler) {
200209
}
201210
});
202211
}
212+
213+
private void addSmartNamePass() {
214+
passes.add(
215+
new PassFactory("smartNamePass", true) {
216+
@Override
217+
protected CompilerPass create(final AbstractCompiler compiler) {
218+
return new CompilerPass() {
219+
@Override
220+
public void process(Node externs, Node root) {
221+
NameAnalyzer na = new NameAnalyzer(compiler, false);
222+
na.process(externs, root);
223+
na.removeUnreferenced();
224+
}
225+
};
226+
}
227+
});
228+
}
203229
}

0 commit comments

Comments
 (0)