Skip to content

Commit

Permalink
Do not alias this in default parameters of async generator functions.
Browse files Browse the repository at this point in the history 
Fixes http://github.com/google/closure-compiler/issues/3178

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=240836570
  • Loading branch information
@brad4d @tjgq
brad4d authored and tjgq committed Mar 28, 2019
1 parent 2a2e7a2 commit 83a8b02
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 26 deletions.
84 changes: 62 additions & 22 deletions src/com/google/javascript/jscomp/RewriteAsyncIteration.java
View file
Expand Up @@ -106,43 +106,80 @@ public final class RewriteAsyncIteration implements NodeTraversal.Callback, HotS
*/
private static final class LexicalContext {

// Node that creates the context
private final Node contextRoot;
// The current function, or null if root scope where we are not in a function.
private final Node function;
// The context of the most recent definition of this/super/arguments
private final ThisSuperArgsContext thisSuperArgsContext;

// Represents the global/root scope. Should only exist on the bottom of the contextStack.
private LexicalContext() {
private LexicalContext(Node contextRoot) {
this.contextRoot = checkNotNull(contextRoot);
this.function = null;
this.thisSuperArgsContext = null;
}

private LexicalContext(LexicalContext parent, Node function) {
/**
* Represents the context of a function or its parameter list.
*
* @param parent enclosing context
* @param contextRoot FUNCTION or PARAM_LIST node
* @param function same as contextRoot or the FUNCTION containing the PARAM_LIST
*/
private LexicalContext(LexicalContext parent, Node contextRoot, Node function) {
checkNotNull(parent);
checkNotNull(contextRoot);
checkArgument(contextRoot == function || contextRoot.isParamList(), contextRoot);
checkNotNull(function);
checkArgument(function.isFunction());
checkArgument(function.isFunction(), function);
this.contextRoot = contextRoot;
this.function = function;

if (function.isArrowFunction()) {
// Use the parent context to inherit this, arguments, and super.
// Use the parent context to inherit this, arguments, and super for an arrow function or its
// parameter list.
this.thisSuperArgsContext = parent.thisSuperArgsContext;
} else {
// Non-arrow gets its own context defining this, arguments, and super.
} else if (contextRoot.isFunction()) {
// Non-arrow function gets its own context defining `this`, `arguments`, and `super`.
this.thisSuperArgsContext = new ThisSuperArgsContext(this);
} else {
// contextRoot is a parameter list.
// Never alias `this`, `arguments`, or `super` for normal function parameter lists.
// They are implicitly defined there.
this.thisSuperArgsContext = null;
}
}

static LexicalContext newGlobalContext() {
return new LexicalContext();
static LexicalContext newGlobalContext(Node contextRoot) {
return new LexicalContext(contextRoot);
}

static LexicalContext newContextForFunction(LexicalContext parent, Node function) {
return new LexicalContext(parent, function);
// Functions need their own context because:
// - async generator functions must be transpiled
// - non-async generator functions must NOT be transpiled
// - arrow functions inside of async generator functions need to have
// `this`, `arguments`, and `super` references aliased, including in their
// parameter lists
return new LexicalContext(parent, function, function);
}

static LexicalContext newContextForParamList(LexicalContext parent, Node paramList) {
// Parameter lists need their own context because `this`, `arguments`, and `super` must NOT be
// aliased for non-arrow function parameter lists, even for async generator functions.
return new LexicalContext(parent, paramList, parent.function);
}

Node getFunctionDeclaringThisArgsSuper() {
return thisSuperArgsContext.ctx.function;
}

/** Is it necessary to replace `this`, `super`, and `arguments` with aliases in this context? */
boolean mustReplaceThisSuperArgs() {
return thisSuperArgsContext != null
&& getFunctionDeclaringThisArgsSuper().isAsyncGeneratorFunction();
}
}

/**
Expand Down Expand Up @@ -224,7 +261,7 @@ public void process(Node externs, Node root) {
*/
private void process(Node root, boolean hotSwap) {
checkState(contextStack.isEmpty());
contextStack.push(LexicalContext.newGlobalContext());
contextStack.push(LexicalContext.newGlobalContext(root));
if (hotSwap) {
TranspilationPasses.hotSwapTranspile(compiler, root, transpiledFeatures, this);
} else {
Expand All @@ -236,15 +273,12 @@ private void process(Node root, boolean hotSwap) {
checkState(contextStack.isEmpty());
}

private boolean isInContextOfAsyncGenerator(LexicalContext ctx) {
return ctx.thisSuperArgsContext != null
&& ctx.getFunctionDeclaringThisArgsSuper().isAsyncGeneratorFunction();
}

@Override
public boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent) {
if (n.isFunction()) {
contextStack.push(LexicalContext.newContextForFunction(contextStack.element(), n));
} else if (n.isParamList()) {
contextStack.push(LexicalContext.newContextForParamList(contextStack.element(), n));
}
return true;
}
Expand All @@ -254,12 +288,18 @@ public void visit(NodeTraversal t, Node n, Node parent) {
LexicalContext ctx = contextStack.element();
switch (n.getToken()) {
// Async Generators (and popping contexts)
case PARAM_LIST:
// Done handling parameter list, so pop its context
checkState(n.equals(ctx.contextRoot), n);
contextStack.pop();
break;
case FUNCTION:
checkState(n.equals(ctx.function));
checkState(n.equals(ctx.contextRoot));
if (n.isAsyncGeneratorFunction()) {
convertAsyncGenerator(t, n);
prependTempVarDeclarations(ctx, t);
}
// Done handling function, so pop its context
contextStack.pop();
break;
case AWAIT:
Expand All @@ -285,17 +325,17 @@ public void visit(NodeTraversal t, Node n, Node parent) {

// Maintaining references to this/arguments/super
case THIS:
if (isInContextOfAsyncGenerator(ctx)) {
if (ctx.mustReplaceThisSuperArgs()) {
replaceThis(t, ctx, n);
}
break;
case NAME:
if (isInContextOfAsyncGenerator(ctx) && n.matchesQualifiedName("arguments")) {
if (ctx.mustReplaceThisSuperArgs() && n.matchesQualifiedName("arguments")) {
replaceArguments(t, ctx, n);
}
break;
case SUPER:
if (isInContextOfAsyncGenerator(ctx)) {
if (ctx.mustReplaceThisSuperArgs()) {
replaceSuper(t, ctx, n, parent);
}
break;
Expand Down Expand Up @@ -569,7 +609,7 @@ private Node constructAwaitNextResult(

private void replaceThis(NodeTraversal t, LexicalContext ctx, Node n) {
checkArgument(n.isThis());
checkArgument(ctx != null && isInContextOfAsyncGenerator(ctx));
checkArgument(ctx != null && ctx.mustReplaceThisSuperArgs());
checkArgument(ctx.function != null, "Cannot prepend declarations to root scope");
checkNotNull(ctx.thisSuperArgsContext);

Expand All @@ -580,7 +620,7 @@ private void replaceThis(NodeTraversal t, LexicalContext ctx, Node n) {

private void replaceArguments(NodeTraversal t, LexicalContext ctx, Node n) {
checkArgument(n.isName() && "arguments".equals(n.getString()));
checkArgument(ctx != null && isInContextOfAsyncGenerator(ctx));
checkArgument(ctx != null && ctx.mustReplaceThisSuperArgs());
checkArgument(ctx.function != null, "Cannot prepend declarations to root scope");
checkNotNull(ctx.thisSuperArgsContext);

Expand All @@ -599,7 +639,7 @@ private void replaceSuper(NodeTraversal t, LexicalContext ctx, Node n, Node pare
return;
}
checkArgument(n.isSuper());
checkArgument(ctx != null && isInContextOfAsyncGenerator(ctx));
checkArgument(ctx != null && ctx.mustReplaceThisSuperArgs());
checkArgument(ctx.function != null, "Cannot prepend declarations to root scope");
checkNotNull(ctx.thisSuperArgsContext);

Expand Down
35 changes: 31 additions & 4 deletions test/com/google/javascript/jscomp/RewriteAsyncIterationTest.java
View file
Expand Up @@ -358,22 +358,49 @@ public void testThisInAsyncGenerator() {
@Test
public void testThisInAsyncGeneratorNestedInAsyncGenerator() {
test(
"async function* baz() { return async function*() { yield this; } }",
lines(
"function baz() {",
"async function* baz(outerT = this) {",
" return async function*(innerT = this) {",
" yield innerT || this;",
" }",
"}"),
lines(
// `this` in parameter list shouldn't be aliased
"function baz(outerT = this) {",
" return new $jscomp.AsyncGeneratorWrapper((function*() {",
" return function() {",
// `this` in parameter list shouldn't be aliased
" return function(innerT = this) {",
" const $jscomp$asyncIter$this = this;",
" return new $jscomp.AsyncGeneratorWrapper((function*() {",
// `this` in body should be aliased
" yield new $jscomp.AsyncGeneratorWrapper$ActionRecord(",
" $jscomp.AsyncGeneratorWrapper$ActionEnum.YIELD_VALUE,",
" $jscomp$asyncIter$this);",
" innerT || $jscomp$asyncIter$this);",
" })());",
" };",
" })());",
"}"));
}

@Test
public void testThisInArrowNestedInAsyncGenerator() {
test(
lines(
"async function* baz() {",
// both instances of `this` musts be changed to aliases
" return (t = this) => t || this;",
"}"),
lines(
"function baz() {",
" const $jscomp$asyncIter$this = this;",
" return new $jscomp.AsyncGeneratorWrapper((function*() {",
" return (t = $jscomp$asyncIter$this) =>",
" t || $jscomp$asyncIter$this;",
" })());",
"}",
""));
}

@Test
public void testThisInFunctionNestedInAsyncGenerator() {
test(
Expand Down

0 comments on commit 83a8b02

Please sign in to comment.