Skip to content

Commit

Permalink
Switch to Es6SyntacticScopeCreator in InlineVariables.
Browse files Browse the repository at this point in the history 
Part of #950

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=155096028
  • Loading branch information
@brad4d
tbreisacher authored and brad4d committed May 5, 2017
1 parent 27039b0 commit c45114f
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 27 deletions.
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/InlineVariables.java
View file
Expand Up @@ -84,7 +84,7 @@ public void process(Node externs, Node root) {
new ReferenceCollectingCallback( new ReferenceCollectingCallback(
compiler, compiler,
new InliningBehavior(), new InliningBehavior(),
SyntacticScopeCreator.makeUntyped(compiler), new Es6SyntacticScopeCreator(compiler),
getFilterForMode()); getFilterForMode());
callback.process(externs, root); callback.process(externs, root);
} }
Expand Down
12 changes: 10 additions & 2 deletions src/com/google/javascript/jscomp/ReferenceCollectingCallback.java
View file
Expand Up @@ -240,15 +240,22 @@ private void outOfBandTraversal(Var v) {
public void enterScope(NodeTraversal t) { public void enterScope(NodeTraversal t) {
Node n = t.getScopeRoot(); Node n = t.getScopeRoot();
BasicBlock parent = blockStack.isEmpty() ? null : peek(blockStack); 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));
}
} }


/** /**
* Updates block stack and invokes any additional behavior. * Updates block stack and invokes any additional behavior.
*/ */
@Override @Override
public void exitScope(NodeTraversal t) { public void exitScope(NodeTraversal t) {
pop(blockStack); if (t.getScope().isHoistScope()) {
pop(blockStack);
}
if (t.inGlobalScope()) { if (t.inGlobalScope()) {
// Update global scope reference lists when we are done with it. // Update global scope reference lists when we are done with it.
compiler.updateGlobalVarReferences(referenceMap, t.getScopeRoot()); compiler.updateGlobalVarReferences(referenceMap, t.getScopeRoot());
Expand Down Expand Up @@ -845,6 +852,7 @@ static final class BasicBlock {
pType == Token.DO pType == Token.DO
|| pType == Token.WHILE || pType == Token.WHILE
|| pType == Token.FOR || pType == Token.FOR
|| pType == Token.FOR_OF
|| pType == Token.FOR_IN; || pType == Token.FOR_IN;
} else { } else {
this.isLoop = false; this.isLoop = false;
Expand Down
88 changes: 85 additions & 3 deletions test/com/google/javascript/jscomp/InlineVariablesTest.java
View file
Expand Up @@ -16,6 +16,7 @@


package com.google.javascript.jscomp; package com.google.javascript.jscomp;


import static com.google.javascript.jscomp.CompilerOptions.LanguageMode.ECMASCRIPT_NEXT;


/** /**
* Verifies that valid candidates for inlining are inlined, but * Verifies that valid candidates for inlining are inlined, but
Expand All @@ -31,6 +32,7 @@ public final class InlineVariablesTest extends CompilerTestCase {


public InlineVariablesTest() { public InlineVariablesTest() {
enableNormalize(); enableNormalize();
setAcceptedLanguage(ECMASCRIPT_NEXT);
} }


@Override @Override
Expand Down Expand Up @@ -825,14 +827,94 @@ public void testInlineSwitchVar() {
"switch (y) {}"); "switch (y) {}");
} }


public void testInlineCatchAlias1() { public void testInlineSwitchLet() {
test( 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( LINE_JOINER.join(
"try {", "try {",
"} catch (e) {", "} catch (e) {",
" var y = e;", " var y = e;",
" g();" , " g();" ,
" y;y;" , " 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( LINE_JOINER.join(
"try {", "try {",
Expand All @@ -842,12 +924,12 @@ public void testInlineCatchAlias1() {
"}")); "}"));
} }


public void testInlineCatchAlias2() { public void testInlineCatchAliasLet2() {
test( test(
LINE_JOINER.join( LINE_JOINER.join(
"try {", "try {",
"} catch (e) {", "} catch (e) {",
" var y; y = e;", " let y; y = e;",
" g();", " g();",
" y;y;", " y;y;",
"}"), "}"),
Expand Down
25 changes: 4 additions & 21 deletions test/com/google/javascript/jscomp/ReferenceCollectingCallbackTest.java
View file
Expand Up @@ -270,29 +270,12 @@ public void afterExitScope(NodeTraversal t, ReferenceMap rm) {
} }


public void testBasicBlocks() { public void testBasicBlocks() {
testBehavior( testBasicBlocks(true);
LINE_JOINER.join( testBasicBlocks(false);
"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);
}
}
});
} }


public void testBasicBlocks_oldScopeCreator() { private void testBasicBlocks(boolean scopeCreator) {
es6ScopeCreator = false; es6ScopeCreator = scopeCreator;
testBehavior( testBehavior(
LINE_JOINER.join( LINE_JOINER.join(
"var x = 0;", "var x = 0;",
Expand Down

0 comments on commit c45114f

Please sign in to comment.