Skip to content

Commit 79891f0

Browse files
ChadKillingsworthblickly
ChadKillingsworth
authored and
blickly
committed
Update async await transpilation to avoid captures of super
Closes #3103 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=217866046
1 parent 45a8658 commit 79891f0

File tree

6 files changed

+234
-22
lines changed

6 files changed

+234
-22
lines changed

‎src/com/google/javascript/jscomp/RewriteAsyncFunctions.java

Lines changed: 72 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -120,16 +120,54 @@ void recordAsyncArgumentsReplacementWasDone() {
120120
}
121121
}
122122

123-
private final Deque<LexicalContext> contextStack;
124-
private final AbstractCompiler compiler;
125123
private static final FeatureSet transpiledFeatures =
126124
FeatureSet.BARE_MINIMUM.with(Feature.ASYNC_FUNCTIONS);
127125

128-
public RewriteAsyncFunctions(AbstractCompiler compiler) {
129-
checkNotNull(compiler);
130-
this.compiler = compiler;
126+
private final Deque<LexicalContext> contextStack;
127+
private final AbstractCompiler compiler;
128+
129+
/**
130+
* If this option is set to true, then this pass will rewrite references to properties using super
131+
* (e.g. `super.method()`) to avoid using `super` within an arrow function.
132+
*
133+
* <p>This option exists due to a bug in MS Edge 17 which causes it to fail to access super
134+
* properties correctly from within arrow functions.
135+
*
136+
* <p>See https://github.com/Microsoft/ChakraCore/issues/5784
137+
*
138+
* <p>If the final compiler output will not include ES6 classes, this option should not be set. It
139+
* isn't needed since the `super` references will be transpiled away anyway. Also, when this
140+
* option is set it uses `Object.getPrototypeOf()` to rewrite `super`, which may not exist in
141+
* pre-ES6 JS environments.
142+
*/
143+
private final boolean rewriteSuperPropertyReferencesWithoutSuper;
144+
145+
private RewriteAsyncFunctions(Builder builder) {
146+
checkNotNull(builder);
147+
this.compiler = builder.compiler;
131148
this.contextStack = new ArrayDeque<>();
132149
this.contextStack.addFirst(new LexicalContext());
150+
this.rewriteSuperPropertyReferencesWithoutSuper =
151+
builder.rewriteSuperPropertyReferencesWithoutSuper;
152+
}
153+
154+
static class Builder {
155+
private final AbstractCompiler compiler;
156+
private boolean rewriteSuperPropertyReferencesWithoutSuper = false;
157+
158+
Builder(AbstractCompiler compiler) {
159+
checkNotNull(compiler);
160+
this.compiler = compiler;
161+
}
162+
163+
Builder rewriteSuperPropertyReferencesWithoutSuper(boolean value) {
164+
rewriteSuperPropertyReferencesWithoutSuper = value;
165+
return this;
166+
}
167+
168+
RewriteAsyncFunctions build() {
169+
return new RewriteAsyncFunctions(this);
170+
}
133171
}
134172

135173
@Override
@@ -244,9 +282,36 @@ private void convertAsyncFunction(NodeTraversal t, LexicalContext functionContex
244282
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.CONST_DECLARATIONS);
245283
}
246284
for (String replacedMethodName : functionContext.replacedSuperProperties) {
285+
Node superReference;
286+
if (rewriteSuperPropertyReferencesWithoutSuper) {
287+
// Rewrite to avoid using `super` within an arrow function.
288+
// See more information on definition of this option.
289+
// TODO(bradfordcsmith): RewriteAsyncIteration and RewriteAsyncFunctions have the
290+
// same logic for dealing with super references. Consider having them share
291+
// it from a common place instead of duplicating.
292+
293+
// static super: Object.getPrototypeOf(this);
294+
superReference =
295+
IR.call(IR.getprop(IR.name("Object"), IR.string("getPrototypeOf")), IR.thisNode());
296+
if (!originalFunction.getParent().isStaticMember()) {
297+
// instance super: Object.getPrototypeOf(Object.getPrototypeOf(this))
298+
superReference =
299+
IR.call(IR.getprop(IR.name("Object"), IR.string("getPrototypeOf")), superReference);
300+
}
301+
} else {
302+
superReference = IR.superNode();
303+
}
304+
247305
// const super$get$x = () => super.x;
248-
Node arrowFunction = IR.arrowFunction(
249-
IR.name(""), IR.paramList(), IR.getprop(IR.superNode(), IR.string(replacedMethodName)));
306+
// OR avoid super for static method (class object -> superclass object)
307+
// const super$get$x = () => Object.getPrototypeOf(this).x
308+
// OR avoid super for instance method (instance -> prototype -> super prototype)
309+
// const super$get$x = () => Object.getPrototypeOf(Object.getPrototypeOf(this)).x
310+
Node arrowFunction =
311+
IR.arrowFunction(
312+
IR.name(""),
313+
IR.paramList(),
314+
IR.getprop(superReference, IR.string(replacedMethodName)));
250315
compiler.reportChangeToChangeScope(arrowFunction);
251316
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.ARROW_FUNCTIONS);
252317

‎src/com/google/javascript/jscomp/RewriteAsyncIteration.java

Lines changed: 65 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,22 @@ public final class RewriteAsyncIteration implements NodeTraversal.Callback, HotS
8080
private final String argumentsVarName = "$jscomp$asyncIter$arguments";
8181
private final String superPropGetterPrefix = "$jscomp$asyncIter$super$get$";
8282

83+
/**
84+
* If this option is set to true, then this pass will rewrite references to properties using super
85+
* (e.g. `super.method()`) to avoid using `super` within an arrow function.
86+
*
87+
* <p>This option exists due to a bug in MS Edge 17 which causes it to fail to access super
88+
* properties correctly from within arrow functions.
89+
*
90+
* <p>See https://github.com/Microsoft/ChakraCore/issues/5784
91+
*
92+
* <p>If the final compiler output will not include ES6 classes, this option should not be set. It
93+
* isn't needed since the `super` references will be transpiled away anyway. Also, when this
94+
* option is set it uses `Object.getPrototypeOf()` to rewrite `super`, which may not exist in
95+
* pre-ES6 JS environments.
96+
*/
97+
private final boolean rewriteSuperPropertyReferencesWithoutSuper;
98+
8399
/**
84100
* Tracks a function and its context of this/arguments/super, if such a context exists.
85101
*/
@@ -142,9 +158,30 @@ private static final class ThisSuperArgsContext {
142158
}
143159
}
144160

145-
RewriteAsyncIteration(AbstractCompiler compiler) {
146-
this.compiler = compiler;
161+
private RewriteAsyncIteration(Builder builder) {
162+
this.compiler = builder.compiler;
147163
this.contextStack = new ArrayDeque<>();
164+
this.rewriteSuperPropertyReferencesWithoutSuper =
165+
builder.rewriteSuperPropertyReferencesWithoutSuper;
166+
}
167+
168+
static class Builder {
169+
private final AbstractCompiler compiler;
170+
private boolean rewriteSuperPropertyReferencesWithoutSuper = false;
171+
172+
Builder(AbstractCompiler compiler) {
173+
checkNotNull(compiler);
174+
this.compiler = compiler;
175+
}
176+
177+
Builder rewriteSuperPropertyReferencesWithoutSuper(boolean value) {
178+
rewriteSuperPropertyReferencesWithoutSuper = value;
179+
return this;
180+
}
181+
182+
RewriteAsyncIteration build() {
183+
return new RewriteAsyncIteration(this);
184+
}
148185
}
149186

150187
@Override
@@ -558,16 +595,36 @@ private void prependTempVarDeclarations(LexicalContext ctx, NodeTraversal t) {
558595
.useSourceInfoFromForTree(block));
559596
}
560597
for (String replacedMethodName : thisSuperArgsCtx.usedSuperProperties) {
561-
// { // prefixBlock
562-
// const $jscomp$asyncIter$this = this;
563-
// const $jscomp$asyncIter$arguments = arguments;
564-
// const $jscomp$asyncIter$super$get$x = () => super.x;
565-
// }
598+
// const super$get$x = () => super.x;
599+
// OR avoid super for static method (class object -> superclass object)
600+
// const super$get$x = () => Object.getPrototypeOf(this).x
601+
// OR avoid super for instance method (instance -> prototype -> super prototype)
602+
// const super$get$x = () => Object.getPrototypeOf(Object.getPrototypeOf(this)).x
603+
Node superReference;
604+
if (rewriteSuperPropertyReferencesWithoutSuper) {
605+
// Rewrite to avoid using `super` within an arrow function.
606+
// See more information on definition of this option.
607+
// TODO(bradfordcsmith): RewriteAsyncIteration and RewriteAsyncFunctions have the
608+
// same logic for dealing with super references. Consider having them share
609+
// it from a common place instead of duplicating.
610+
611+
// static super: Object.getPrototypeOf(this);
612+
superReference =
613+
IR.call(IR.getprop(IR.name("Object"), IR.string("getPrototypeOf")), IR.thisNode());
614+
if (!ctx.function.getParent().isStaticMember()) {
615+
// instance super: Object.getPrototypeOf(Object.getPrototypeOf(this))
616+
superReference =
617+
IR.call(IR.getprop(IR.name("Object"), IR.string("getPrototypeOf")), superReference);
618+
}
619+
} else {
620+
superReference = IR.superNode();
621+
}
622+
566623
Node arrowFunction =
567624
IR.arrowFunction(
568625
IR.name(""),
569626
IR.paramList(),
570-
IR.getprop(IR.superNode(), IR.string(replacedMethodName)));
627+
IR.getprop(superReference, IR.string(replacedMethodName)));
571628
compiler.reportChangeToChangeScope(arrowFunction);
572629
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.ARROW_FUNCTIONS);
573630
String superReplacementName = superPropGetterPrefix + replacedMethodName;

‎src/com/google/javascript/jscomp/TranspilationPasses.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,13 @@ protected FeatureSet featureSet() {
217217
new HotSwapPassFactory("rewriteAsyncFunctions") {
218218
@Override
219219
protected HotSwapCompilerPass create(final AbstractCompiler compiler) {
220-
return new RewriteAsyncFunctions(compiler);
220+
return new RewriteAsyncFunctions.Builder(compiler)
221+
// If ES6 classes will not be transpiled away later,
222+
// transpile away property references that use `super` in async functions.
223+
// See explanation in RewriteAsyncFunctions.
224+
.rewriteSuperPropertyReferencesWithoutSuper(
225+
!compiler.getOptions().needsTranspilationFrom(FeatureSet.ES6))
226+
.build();
221227
}
222228

223229
@Override
@@ -230,7 +236,13 @@ protected FeatureSet featureSet() {
230236
new HotSwapPassFactory("rewriteAsyncIteration") {
231237
@Override
232238
protected HotSwapCompilerPass create(final AbstractCompiler compiler) {
233-
return new RewriteAsyncIteration(compiler);
239+
return new RewriteAsyncIteration.Builder(compiler)
240+
// If ES6 classes will not be transpiled away later,
241+
// transpile away property references that use `super` in async iteration.
242+
// See explanation in RewriteAsyncIteration.
243+
.rewriteSuperPropertyReferencesWithoutSuper(
244+
!compiler.getOptions().needsTranspilationFrom(FeatureSet.ES6))
245+
.build();
234246
}
235247

236248
@Override

‎test/com/google/javascript/jscomp/IntegrationTest.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3988,7 +3988,8 @@ public void testAsyncFunctionSuper() {
39883988
"}",
39893989
"class Baz extends Foo {",
39903990
" bar() {",
3991-
" const $jscomp$async$this = this, $jscomp$async$super$get$bar = () => super.bar;",
3991+
" const $jscomp$async$this = this, $jscomp$async$super$get$bar =",
3992+
" () => Object.getPrototypeOf(Object.getPrototypeOf(this)).bar;",
39923993
" return $jscomp.asyncExecutePromiseGeneratorFunction(function*() {",
39933994
" yield Promise.resolve();",
39943995
" $jscomp$async$super$get$bar().call($jscomp$async$this);",
@@ -4041,7 +4042,8 @@ public void testAsyncIterationSuper() {
40414042
"class Baz extends Foo {",
40424043
" bar() {",
40434044
" const $jscomp$asyncIter$this = this,",
4044-
" $jscomp$asyncIter$super$get$bar = () => super.bar;",
4045+
" $jscomp$asyncIter$super$get$bar =",
4046+
" () => Object.getPrototypeOf(Object.getPrototypeOf(this)).bar;",
40454047
" return new $jscomp.AsyncGeneratorWrapper(function*() {",
40464048
" $jscomp$asyncIter$super$get$bar().call($jscomp$asyncIter$this).next();",
40474049
" }());",

‎test/com/google/javascript/jscomp/RewriteAsyncFunctionsTest.java

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static com.google.common.truth.Truth.assertThat;
1919

2020
import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
21+
import com.google.javascript.jscomp.parsing.parser.FeatureSet;
2122
import org.junit.Before;
2223
import org.junit.Test;
2324
import org.junit.runner.RunWith;
@@ -37,7 +38,10 @@ public void setUp() throws Exception {
3738

3839
@Override
3940
protected CompilerPass getProcessor(Compiler compiler) {
40-
return new RewriteAsyncFunctions(compiler);
41+
return new RewriteAsyncFunctions.Builder(compiler)
42+
.rewriteSuperPropertyReferencesWithoutSuper(
43+
!compiler.getOptions().needsTranspilationFrom(FeatureSet.ES6))
44+
.build();
4145
}
4246

4347
// Don't let the compiler actually inject any code.
@@ -142,6 +146,73 @@ public void testInnerSuperReference() {
142146
"}"));
143147
}
144148

149+
@Test
150+
public void testInnerSuperCallEs2015Out() {
151+
setLanguageOut(LanguageMode.ECMASCRIPT_2015);
152+
test(
153+
lines(
154+
"class A {",
155+
" m() {",
156+
" return this;",
157+
" }",
158+
"}",
159+
"class X extends A {",
160+
" async m() {",
161+
" return super.m();",
162+
" }",
163+
"}"),
164+
lines(
165+
"class A {",
166+
" m() {",
167+
" return this;",
168+
" }",
169+
"}",
170+
"class X extends A {",
171+
" m() {",
172+
" const $jscomp$async$this = this;",
173+
" const $jscomp$async$super$get$m =",
174+
" () => Object.getPrototypeOf(Object.getPrototypeOf(this)).m;",
175+
" return $jscomp.asyncExecutePromiseGeneratorFunction(",
176+
" function* () {",
177+
" return $jscomp$async$super$get$m().call($jscomp$async$this);",
178+
" });",
179+
" }",
180+
"}"));
181+
}
182+
183+
@Test
184+
public void testInnerSuperCallStaticEs2015Out() {
185+
setLanguageOut(LanguageMode.ECMASCRIPT_2015);
186+
test(
187+
lines(
188+
"class A {",
189+
" static m() {",
190+
" return this;",
191+
" }",
192+
"}",
193+
"class X extends A {",
194+
" static async m() {",
195+
" return super.m();",
196+
" }",
197+
"}"),
198+
lines(
199+
"class A {",
200+
" static m() {",
201+
" return this;",
202+
" }",
203+
"}",
204+
"class X extends A {",
205+
" static m() {",
206+
" const $jscomp$async$this = this;",
207+
" const $jscomp$async$super$get$m = () => Object.getPrototypeOf(this).m;",
208+
" return $jscomp.asyncExecutePromiseGeneratorFunction(",
209+
" function* () {",
210+
" return $jscomp$async$super$get$m().call($jscomp$async$this);",
211+
" });",
212+
" }",
213+
"}"));
214+
}
215+
145216
@Test
146217
public void testNestedArrowFunctionUsingThis() {
147218
test(

‎test/com/google/javascript/jscomp/RewriteAsyncIterationTest.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package com.google.javascript.jscomp;
1717

1818
import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
19+
import com.google.javascript.jscomp.parsing.parser.FeatureSet;
1920
import org.junit.Before;
2021
import org.junit.Test;
2122
import org.junit.runner.RunWith;
@@ -47,7 +48,10 @@ protected CompilerOptions getOptions() {
4748

4849
@Override
4950
protected CompilerPass getProcessor(Compiler compiler) {
50-
return new RewriteAsyncIteration(compiler);
51+
return new RewriteAsyncIteration.Builder(compiler)
52+
.rewriteSuperPropertyReferencesWithoutSuper(
53+
!compiler.getOptions().needsTranspilationFrom(FeatureSet.ES6))
54+
.build();
5155
}
5256

5357
@Test
@@ -225,7 +229,8 @@ public void testInnerSuperReferenceInAsyncGenerator() {
225229
"}",
226230
"class X extends A {",
227231
" m() {",
228-
" const $jscomp$asyncIter$super$get$m = () => super.m;",
232+
" const $jscomp$asyncIter$super$get$m =",
233+
" () => Object.getPrototypeOf(Object.getPrototypeOf(this)).m;",
229234
" return new $jscomp.AsyncGeneratorWrapper(",
230235
" function* () {",
231236
" const tmp = $jscomp$asyncIter$super$get$m();",

0 commit comments

Comments
 (0)