Skip to content

Commit 07080fa

Browse files
rishipalcopybara-github
rishipal
authored and
copybara-github
committed
Give unique names to the new $jscomp$super$this and $jscomp$tmp$error variables created during class rewriting.
This allows moving the class rewriting pass post normalize without breaking normalization (i.e. by preserving globally unique name for every var). PiperOrigin-RevId: 621239254
1 parent a381b20 commit 07080fa

File tree

4 files changed

+112
-76
lines changed

4 files changed

+112
-76
lines changed

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

+45-20
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,14 @@ private static final class ConstructorData {
5959
// meaning compiler.getTranspilationNamespace() does not work.
6060
private GlobalNamespace globalNamespace;
6161
private final StaticScope transpilationNamespace;
62+
private final UniqueIdSupplier uniqueIdSupplier;
6263

6364
public Es6ConvertSuperConstructorCalls(AbstractCompiler compiler) {
6465
this.compiler = compiler;
6566
this.astFactory = compiler.createAstFactory();
6667
this.transpilationNamespace = compiler.getTranspilationNamespace();
6768
this.constructorDataStack = new ArrayDeque<>();
69+
this.uniqueIdSupplier = compiler.getUniqueIdSupplier();
6870
}
6971

7072
@Override
@@ -109,6 +111,14 @@ private void visitSuper(NodeTraversal t, ConstructorData constructorData) {
109111
if (superCalls.isEmpty()) {
110112
return; // nothing to do
111113
}
114+
115+
// Give a unique name to the $jscomp$super$this variables created when rewriting this super to
116+
// preserve normalization.
117+
String uniqueSuperThisName =
118+
SUPER_THIS
119+
+ "$"
120+
+ uniqueIdSupplier.getUniqueId(compiler.getInput(NodeUtil.getInputId(constructor)));
121+
112122
if (constructor.isFromExterns()) {
113123
// This class is defined in an externs file, so it's only a stub, not the actual
114124
// implementation that should be instantiated.
@@ -142,14 +152,14 @@ private void visitSuper(NodeTraversal t, ConstructorData constructorData) {
142152
// To correctly extend them with the ES5 classes we're generating here, we must use
143153
// `$jscomp.construct`, which is our wrapper around `Reflect.construct`.
144154
convertSuperCallsToJsCompConstructCalls(
145-
constructor, superCalls, superClassNameNode, thisType);
155+
constructor, superCalls, superClassNameNode, thisType, uniqueSuperThisName);
146156
} else if (isNativeErrorClass(t, superClassQName)) {
147157
// TODO(bradfordcsmith): It might be better to use $jscomp.construct() for these instead
148158
// of our custom-made, Error-specific workaround.
149159
for (Node superCall : superCalls) {
150160
Node newSuperCall =
151161
createNewSuperCall(superClassNameNode, superCall, thisType, type(superCall));
152-
replaceNativeErrorSuperCall(superCall, newSuperCall);
162+
replaceNativeErrorSuperCall(superCall, newSuperCall, t.getInput());
153163
}
154164
} else if (isKnownToReturnOnlyUndefined(superClassQName)) {
155165
// super() will not change the value of `this`.
@@ -207,13 +217,14 @@ private void visitSuper(NodeTraversal t, ConstructorData constructorData) {
207217
final AstFactory.Type typeOfThis = getTypeOfThisForConstructor(constructor);
208218
// `this` -> `$jscomp$super$this` throughout the constructor body,
209219
// except for super() calls.
210-
updateThisToSuperThis(typeOfThis, constructorBody, superCalls);
220+
updateThisToSuperThis(typeOfThis, constructorBody, superCalls, uniqueSuperThisName);
211221
// Start constructor with `var $jscomp$super$this;`
212222
constructorBody.addChildToFront(
213-
IR.var(astFactory.createName(SUPER_THIS, typeOfThis)).srcrefTree(constructorBody));
223+
IR.var(astFactory.createName(uniqueSuperThisName, typeOfThis))
224+
.srcrefTree(constructorBody));
214225
// End constructor with `return $jscomp$super$this;`
215226
constructorBody.addChildToBack(
216-
IR.returnNode(astFactory.createName(SUPER_THIS, typeOfThis))
227+
IR.returnNode(astFactory.createName(uniqueSuperThisName, typeOfThis))
217228
.srcrefTree(constructorBody));
218229
// Replace each super() call with `($jscomp$super$this = <newSuperCall> || this)`
219230
for (Node superCall : superCalls) {
@@ -223,7 +234,7 @@ private void visitSuper(NodeTraversal t, ConstructorData constructorData) {
223234
superCall.replaceWith(
224235
astFactory
225236
.createAssign(
226-
astFactory.createName(SUPER_THIS, typeOfThis),
237+
astFactory.createName(uniqueSuperThisName, typeOfThis),
227238
astFactory.createOr(newSuperCall, astFactory.createThis(typeOfThis)))
228239
.srcrefTreeIfMissing(superCall));
229240
}
@@ -255,7 +266,11 @@ private void visitSuper(NodeTraversal t, ConstructorData constructorData) {
255266
* </code></pre>
256267
*/
257268
private void convertSuperCallsToJsCompConstructCalls(
258-
Node constructor, List<Node> superCalls, Node superClassNameNode, AstFactory.Type thisType) {
269+
Node constructor,
270+
List<Node> superCalls,
271+
Node superClassNameNode,
272+
AstFactory.Type thisType,
273+
String uniqueSuperThisName) {
259274
Node constructorBody = checkNotNull(constructor.getChildAtIndex(2));
260275
Node firstStatement = constructorBody.getFirstChild();
261276
// A constructor body with no call to `super()` is a syntax error for a class that has an
@@ -278,21 +293,23 @@ private void convertSuperCallsToJsCompConstructCalls(
278293
final AstFactory.Type typeOfThis = getTypeOfThisForConstructor(constructor);
279294
// `this` -> `$jscomp$super$this` throughout the constructor body,
280295
// except for super() calls.
281-
updateThisToSuperThis(typeOfThis, constructorBody, superCalls);
296+
updateThisToSuperThis(typeOfThis, constructorBody, superCalls, uniqueSuperThisName);
282297
// Start constructor with `var $jscomp$super$this;`
283298
constructorBody.addChildToFront(
284-
astFactory.createSingleVarNameDeclaration(SUPER_THIS).srcrefTree(constructorBody));
299+
astFactory
300+
.createSingleVarNameDeclaration(uniqueSuperThisName)
301+
.srcrefTree(constructorBody));
285302
// End constructor with `return $jscomp$super$this;`
286303
constructorBody.addChildToBack(
287304
astFactory
288-
.createReturn(astFactory.createName(SUPER_THIS, typeOfThis))
305+
.createReturn(astFactory.createName(uniqueSuperThisName, typeOfThis))
289306
.srcrefTree(constructorBody));
290307
// Replace each super() call with `($jscomp$super$this = $jscomp.construct(...))`
291308
for (Node superCall : superCalls) {
292309
superCall.replaceWith(
293310
astFactory
294311
.createAssign(
295-
astFactory.createName(SUPER_THIS, typeOfThis).srcref(superCall),
312+
astFactory.createName(uniqueSuperThisName, typeOfThis).srcref(superCall),
296313
createJSCompConstructorCall(superClassNameNode, superCall, thisType))
297314
.srcref(superCall));
298315
}
@@ -534,7 +551,8 @@ private static boolean isSpreadOfArguments(Node node) {
534551
return node.isSpread() && node.getOnlyChild().matchesName("arguments");
535552
}
536553

537-
private void replaceNativeErrorSuperCall(Node superCall, Node newSuperCall) {
554+
private void replaceNativeErrorSuperCall(
555+
Node superCall, Node newSuperCall, CompilerInput compilerInput) {
538556
// The native error class constructors always return a new object instead of initializing
539557
// `this`, so a workaround is needed.
540558
Node superStatement = NodeUtil.getEnclosingStatement(superCall);
@@ -543,23 +561,27 @@ private void replaceNativeErrorSuperCall(Node superCall, Node newSuperCall) {
543561

544562
AstFactory.Type thisType = type(newSuperCall);
545563

564+
// Give a unique name to the $jscomp$tmp$error; variables created when rewriting this super to
565+
// preserve normalization.
566+
String tmpErrorName = TMP_ERROR + "$" + uniqueIdSupplier.getUniqueId(compilerInput);
567+
546568
// var $jscomp$tmp$error;
547569
Node getError =
548-
IR.var(astFactory.createName(TMP_ERROR, thisType)).srcrefTreeIfMissing(superCall);
570+
IR.var(astFactory.createName(tmpErrorName, thisType)).srcrefTreeIfMissing(superCall);
549571
getError.insertBefore(superStatement);
550572

551573
// Create an expression to initialize `this` from temporary Error object at the point
552574
// where super.apply() was called.
553575
// $jscomp$tmp$error = Error.call(this, ...),
554576
Node getTmpError =
555-
astFactory.createAssign(astFactory.createName(TMP_ERROR, thisType), newSuperCall);
577+
astFactory.createAssign(astFactory.createName(tmpErrorName, thisType), newSuperCall);
556578
// this.message = $jscomp$tmp$error.message,
557579
Node copyMessage =
558580
astFactory.createAssign(
559581
astFactory.createGetProp(
560582
astFactory.createThis(thisType), "message", type(StandardColors.STRING)),
561583
astFactory.createGetProp(
562-
astFactory.createName(TMP_ERROR, thisType),
584+
astFactory.createName(tmpErrorName, thisType),
563585
"message",
564586
type(StandardColors.STRING)));
565587

@@ -569,12 +591,12 @@ private void replaceNativeErrorSuperCall(Node superCall, Node newSuperCall) {
569591
Node setStack =
570592
astFactory.createAnd(
571593
astFactory.createIn(
572-
astFactory.createString("stack"), astFactory.createName(TMP_ERROR, thisType)),
594+
astFactory.createString("stack"), astFactory.createName(tmpErrorName, thisType)),
573595
astFactory.createAssign(
574596
astFactory.createGetProp(
575597
astFactory.createThis(thisType), "stack", type(StandardColors.STRING)),
576598
astFactory.createGetProp(
577-
astFactory.createName(TMP_ERROR, thisType),
599+
astFactory.createName(tmpErrorName, thisType),
578600
"stack",
579601
type(StandardColors.STRING))));
580602
Node superErrorExpr =
@@ -666,7 +688,10 @@ private boolean isDefinedInSources(NodeTraversal t, String varName) {
666688
}
667689

668690
private void updateThisToSuperThis(
669-
final AstFactory.Type typeOfThis, Node constructorBody, final List<Node> superCalls) {
691+
final AstFactory.Type typeOfThis,
692+
Node constructorBody,
693+
final List<Node> superCalls,
694+
final String uniqueSuperThisName) {
670695
NodeTraversal.Callback replaceThisWithSuperThis =
671696
new NodeTraversal.Callback() {
672697
@Override
@@ -684,11 +709,11 @@ public boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent)
684709
@Override
685710
public void visit(NodeTraversal t, Node n, Node parent) {
686711
if (n.isThis()) {
687-
Node superThis = astFactory.createName(SUPER_THIS, type(n)).srcref(n);
712+
Node superThis = astFactory.createName(uniqueSuperThisName, type(n)).srcref(n);
688713
n.replaceWith(superThis);
689714
} else if (n.isReturn() && !n.hasChildren()) {
690715
// An empty return needs to be changed to return $jscomp$super$this
691-
n.addChildToFront(astFactory.createName(SUPER_THIS, typeOfThis).srcref(n));
716+
n.addChildToFront(astFactory.createName(uniqueSuperThisName, typeOfThis).srcref(n));
692717
}
693718
}
694719
};

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

+36-29
Original file line numberDiff line numberDiff line change
@@ -756,10 +756,11 @@ public void testExtendNativeErrorWithAutogeneratedConstructor() {
756756
"/** @constructor",
757757
" */",
758758
"let C = function() {",
759-
" var $jscomp$tmp$error;",
760-
" $jscomp$tmp$error = Error.apply(this, arguments),",
761-
" this.message = $jscomp$tmp$error.message,",
762-
" ('stack' in $jscomp$tmp$error) && (this.stack = $jscomp$tmp$error.stack),",
759+
" var $jscomp$tmp$error$m1146332801$1;",
760+
" $jscomp$tmp$error$m1146332801$1 = Error.apply(this, arguments),",
761+
" this.message = $jscomp$tmp$error$m1146332801$1.message,",
762+
" ('stack' in $jscomp$tmp$error$m1146332801$1) && (this.stack ="
763+
+ " $jscomp$tmp$error$m1146332801$1.stack),",
763764
" this;",
764765
"};",
765766
"$jscomp.inherits(C, Error);"));
@@ -774,10 +775,11 @@ public void testExtendNativeAggregateErrorWithAutogeneratedConstructor() {
774775
"/** @constructor",
775776
" */",
776777
"let C = function() {",
777-
" var $jscomp$tmp$error;",
778-
" $jscomp$tmp$error = AggregateError.apply(this, arguments),",
779-
" this.message = $jscomp$tmp$error.message,",
780-
" ('stack' in $jscomp$tmp$error) && (this.stack = $jscomp$tmp$error.stack),",
778+
" var $jscomp$tmp$error$m1146332801$1;",
779+
" $jscomp$tmp$error$m1146332801$1 = AggregateError.apply(this, arguments),",
780+
" this.message = $jscomp$tmp$error$m1146332801$1.message,",
781+
" ('stack' in $jscomp$tmp$error$m1146332801$1) && (this.stack ="
782+
+ " $jscomp$tmp$error$m1146332801$1.stack),",
781783
" this;",
782784
"};",
783785
"$jscomp.inherits(C, AggregateError);"));
@@ -798,11 +800,12 @@ public void testExtendNativeErrorExplicitSuperCall() {
798800
"/** @constructor",
799801
" */",
800802
"let C = function() {",
801-
" var $jscomp$tmp$error;",
803+
" var $jscomp$tmp$error$m1146332801$1;",
802804
" var self =",
803-
" ($jscomp$tmp$error = Error.call(this, 'C error'),",
804-
" this.message = $jscomp$tmp$error.message,",
805-
" ('stack' in $jscomp$tmp$error) && (this.stack = $jscomp$tmp$error.stack),",
805+
" ($jscomp$tmp$error$m1146332801$1 = Error.call(this, 'C error'),",
806+
" this.message = $jscomp$tmp$error$m1146332801$1.message,",
807+
" ('stack' in $jscomp$tmp$error$m1146332801$1) && (this.stack ="
808+
+ " $jscomp$tmp$error$m1146332801$1.stack),",
806809
" this)",
807810
" || this;",
808811
"};",
@@ -825,11 +828,13 @@ public void testExtendNativeAggregateErrorExplicitSuperCall() {
825828
"/** @constructor",
826829
" */",
827830
"let C = function() {",
828-
" var $jscomp$tmp$error;",
831+
" var $jscomp$tmp$error$m1146332801$1;",
829832
" var self =",
830-
" ($jscomp$tmp$error = AggregateError.call(this, [new Error('msg')]),",
831-
" this.message = $jscomp$tmp$error.message,",
832-
" ('stack' in $jscomp$tmp$error) && (this.stack = $jscomp$tmp$error.stack),",
833+
" ($jscomp$tmp$error$m1146332801$1 = AggregateError.call(this, [new"
834+
+ " Error('msg')]),",
835+
" this.message = $jscomp$tmp$error$m1146332801$1.message,",
836+
" ('stack' in $jscomp$tmp$error$m1146332801$1) && (this.stack ="
837+
+ " $jscomp$tmp$error$m1146332801$1.stack),",
833838
" this)",
834839
" || this;",
835840
"};",
@@ -1145,10 +1150,11 @@ public void testSuperMightChangeThis() {
11451150
"goog.forwardDeclare('D');",
11461151
"/** @constructor */",
11471152
"let C = function(strArg, n) {",
1148-
" var $jscomp$super$this;",
1149-
" ($jscomp$super$this = D.call(this,strArg) || this).n = n;",
1150-
" return $jscomp$super$this;", // Duplicate because of existing return statement.
1151-
" return $jscomp$super$this;",
1153+
" var $jscomp$super$this$m1146332801$0;",
1154+
" ($jscomp$super$this$m1146332801$0 = D.call(this,strArg) || this).n = n;",
1155+
" return $jscomp$super$this$m1146332801$0;", // Duplicate because of existing return
1156+
// statement.
1157+
" return $jscomp$super$this$m1146332801$0;",
11521158
"}",
11531159
"$jscomp.inherits(C, D);"));
11541160
}
@@ -1216,14 +1222,14 @@ public void testAlternativeSuperCalls_withUnknkownSuperclass() {
12161222
"goog.forwardDeclare('D');",
12171223
"/** @constructor */",
12181224
"let C = function(strArg, n) {",
1219-
" var $jscomp$super$this;",
1225+
" var $jscomp$super$this$m1146332801$0;",
12201226
" if (n >= 0) {",
1221-
" $jscomp$super$this = D.call(this, 'positive: ' + strArg) || this;",
1227+
" $jscomp$super$this$m1146332801$0 = D.call(this, 'positive: ' + strArg) || this;",
12221228
" } else {",
1223-
" $jscomp$super$this = D.call(this, 'negative: ' + strArg) || this;",
1229+
" $jscomp$super$this$m1146332801$0 = D.call(this, 'negative: ' + strArg) || this;",
12241230
" }",
1225-
" $jscomp$super$this.n = n;",
1226-
" return $jscomp$super$this;",
1231+
" $jscomp$super$this$m1146332801$0.n = n;",
1232+
" return $jscomp$super$this$m1146332801$0;",
12271233
"}",
12281234
"$jscomp.inherits(C, D);"));
12291235
}
@@ -1428,10 +1434,11 @@ public void testExtendNativeClass_withExplicitConstructor() {
14281434
" * @constructor",
14291435
" */",
14301436
"let FooPromise = function(callbackArg, msg) {",
1431-
" var $jscomp$super$this;",
1432-
" $jscomp$super$this = $jscomp.construct(Promise, [callbackArg], this.constructor)",
1433-
" $jscomp$super$this.msg = msg;",
1434-
" return $jscomp$super$this;",
1437+
" var $jscomp$super$this$m1146332801$0;",
1438+
" $jscomp$super$this$m1146332801$0 = $jscomp.construct(Promise, [callbackArg],"
1439+
+ " this.constructor)",
1440+
" $jscomp$super$this$m1146332801$0.msg = msg;",
1441+
" return $jscomp$super$this$m1146332801$0;",
14351442
"}",
14361443
"$jscomp.inherits(FooPromise, Promise);",
14371444
""));

0 commit comments

Comments
 (0)