diff --git a/src/com/google/javascript/jscomp/InlineVariables.java b/src/com/google/javascript/jscomp/InlineVariables.java index 519ba06e356..4f02ffc2df9 100644 --- a/src/com/google/javascript/jscomp/InlineVariables.java +++ b/src/com/google/javascript/jscomp/InlineVariables.java @@ -84,7 +84,7 @@ public void process(Node externs, Node root) { new ReferenceCollectingCallback( compiler, new InliningBehavior(), - SyntacticScopeCreator.makeUntyped(compiler), + new Es6SyntacticScopeCreator(compiler), getFilterForMode()); callback.process(externs, root); } diff --git a/src/com/google/javascript/jscomp/ReferenceCollectingCallback.java b/src/com/google/javascript/jscomp/ReferenceCollectingCallback.java index af7002becf2..87a0fba0cb7 100644 --- a/src/com/google/javascript/jscomp/ReferenceCollectingCallback.java +++ b/src/com/google/javascript/jscomp/ReferenceCollectingCallback.java @@ -240,7 +240,12 @@ private void outOfBandTraversal(Var v) { public void enterScope(NodeTraversal t) { Node n = t.getScopeRoot(); BasicBlock parent = blockStack.isEmpty() ? null : peek(blockStack); - blockStack.add(new BasicBlock(parent, n)); + // Don't add all ES6 scope roots to blockStack, only those that are also scopes according to + // the ES5 scoping rules. Other nodes that ought to be considered the root of a BasicBlock + // are added in shouldTraverse() and removed in visit(). + if (t.getScope().isHoistScope()) { + blockStack.add(new BasicBlock(parent, n)); + } } /** @@ -248,7 +253,9 @@ public void enterScope(NodeTraversal t) { */ @Override public void exitScope(NodeTraversal t) { - pop(blockStack); + if (t.getScope().isHoistScope()) { + pop(blockStack); + } if (t.inGlobalScope()) { // Update global scope reference lists when we are done with it. compiler.updateGlobalVarReferences(referenceMap, t.getScopeRoot()); @@ -845,6 +852,7 @@ static final class BasicBlock { pType == Token.DO || pType == Token.WHILE || pType == Token.FOR + || pType == Token.FOR_OF || pType == Token.FOR_IN; } else { this.isLoop = false; diff --git a/test/com/google/javascript/jscomp/InlineVariablesTest.java b/test/com/google/javascript/jscomp/InlineVariablesTest.java index fe612a8910f..6012eb57e7f 100644 --- a/test/com/google/javascript/jscomp/InlineVariablesTest.java +++ b/test/com/google/javascript/jscomp/InlineVariablesTest.java @@ -16,6 +16,7 @@ package com.google.javascript.jscomp; +import static com.google.javascript.jscomp.CompilerOptions.LanguageMode.ECMASCRIPT_NEXT; /** * Verifies that valid candidates for inlining are inlined, but @@ -31,6 +32,7 @@ public final class InlineVariablesTest extends CompilerTestCase { public InlineVariablesTest() { enableNormalize(); + setAcceptedLanguage(ECMASCRIPT_NEXT); } @Override @@ -825,14 +827,94 @@ public void testInlineSwitchVar() { "switch (y) {}"); } - public void testInlineCatchAlias1() { + public void testInlineSwitchLet() { test( + "let x = y; switch (x) {}", + "switch (y) {}"); + } + + // Successfully inlines 'values' and 'e' + public void testInlineIntoForLoop1() { + test( + LINE_JOINER.join( + "function calculate_hashCode() {", + " var values = [1, 2, 3, 4, 5];", + " var hashCode = 1;", + " for (var $array = values, i = 0; i < $array.length; i++) {", + " var e = $array[i];", + " hashCode = 31 * hashCode + calculate_hashCode(e);", + " }", + " return hashCode;", + "}"), + LINE_JOINER.join( + "function calculate_hashCode() {", + " var hashCode = 1;", + " var $array = [1, 2, 3, 4, 5];", + " var i = 0;", + " for (; i < $array.length; i++) {", + " hashCode = 31 * hashCode + calculate_hashCode($array[i]);", + " }", + " return hashCode;", + "}")); + } + + // Inlines 'e' but fails to inline 'values' + // TODO(tbreisacher): Investigate and see if we can improve this. + public void testInlineIntoForLoop2() { + test( + LINE_JOINER.join( + "function calculate_hashCode() {", + " let values = [1, 2, 3, 4, 5];", + " let hashCode = 1;", + " for (let $array = values, i = 0; i < $array.length; i++) {", + " let e = $array[i];", + " hashCode = 31 * hashCode + calculate_hashCode(e);", + " }", + " return hashCode;", + "}"), + LINE_JOINER.join( + "function calculate_hashCode() {", + " let values = [1, 2, 3, 4, 5];", + " let hashCode = 1;", + " for (let $array = values, i = 0; i < $array.length; i++) {", + " hashCode = 31 * hashCode + calculate_hashCode($array[i]);", + " }", + " return hashCode;", + "}")); + } + + // This used to be inlined, but regressed when we switched to the ES6 scope creator. + public void testNoInlineCatchAliasVar1() { + testSame( LINE_JOINER.join( "try {", "} catch (e) {", " var y = e;", " g();" , " y;y;" , + "}")); + } + + // This used to be inlined, but regressed when we switched to the ES6 scope creator. + public void testNoInlineCatchAliasVar2() { + testSame( + LINE_JOINER.join( + "try {", + "} catch (e) {", + " var y; y = e;", + " g();", + " y;y;", + "}")); + } + + public void testInlineCatchAliasLet1() { + test( + LINE_JOINER.join( + "try {", + "} catch (e) {", + " let y = e;", + " g();" , + " y;y;" , "}"), LINE_JOINER.join( "try {", @@ -842,12 +924,12 @@ public void testInlineCatchAlias1() { "}")); } - public void testInlineCatchAlias2() { + public void testInlineCatchAliasLet2() { test( LINE_JOINER.join( "try {", "} catch (e) {", - " var y; y = e;", + " let y; y = e;", " g();", " y;y;", "}"), diff --git a/test/com/google/javascript/jscomp/ReferenceCollectingCallbackTest.java b/test/com/google/javascript/jscomp/ReferenceCollectingCallbackTest.java index c534a30e703..1009e81ffaf 100644 --- a/test/com/google/javascript/jscomp/ReferenceCollectingCallbackTest.java +++ b/test/com/google/javascript/jscomp/ReferenceCollectingCallbackTest.java @@ -270,29 +270,12 @@ public void afterExitScope(NodeTraversal t, ReferenceMap rm) { } public void testBasicBlocks() { - testBehavior( - LINE_JOINER.join( - "var x = 0;", - "switch (x) {", - " case 0:", - " x;", - "}"), - new Behavior() { - @Override - public void afterExitScope(NodeTraversal t, ReferenceMap rm) { - if (t.getScope().isGlobal()) { - ReferenceCollection x = rm.getReferences(t.getScope().getVar("x")); - assertThat(x.references).hasSize(3); - assertNode(x.references.get(0).getBasicBlock().getRoot()).hasType(Token.ROOT); - assertNode(x.references.get(1).getBasicBlock().getRoot()).hasType(Token.SWITCH); - assertNode(x.references.get(2).getBasicBlock().getRoot()).hasType(Token.CASE); - } - } - }); + testBasicBlocks(true); + testBasicBlocks(false); } - public void testBasicBlocks_oldScopeCreator() { - es6ScopeCreator = false; + private void testBasicBlocks(boolean scopeCreator) { + es6ScopeCreator = scopeCreator; testBehavior( LINE_JOINER.join( "var x = 0;",