Skip to content

Commit 34d4206

Browse files
weswighamweswigham
committed
use TransformFlags to optimize and correct super call transforms
1 parent 376ad28 commit 34d4206

16 files changed

+568
-43
lines changed

‎src/compiler/binder.ts

Lines changed: 46 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2734,6 +2734,9 @@ namespace ts {
27342734
case SyntaxKind.PropertyAccessExpression:
27352735
return computePropertyAccess(<PropertyAccessExpression>node, subtreeFlags);
27362736

2737+
case SyntaxKind.ElementAccessExpression:
2738+
return computeElementAccess(<ElementAccessExpression>node, subtreeFlags);
2739+
27372740
default:
27382741
return computeOther(node, kind, subtreeFlags);
27392742
}
@@ -2748,10 +2751,15 @@ namespace ts {
27482751
}
27492752

27502753
if (subtreeFlags & TransformFlags.ContainsSpread
2751-
|| isSuperOrSuperProperty(expression)) {
2754+
|| (expression.transformFlags & (TransformFlags.Super | TransformFlags.ContainsSuper))) {
27522755
// If the this node contains a SpreadExpression, or is a super call, then it is an ES6
27532756
// node.
27542757
transformFlags |= TransformFlags.AssertES2015;
2758+
// super property or element accesses could be inside lambdas, etc, and need a captured `this`,
2759+
// while super keyword for super calls (indicated by TransformFlags.Super) does not (since it can only be top-level in a constructor)
2760+
if (expression.transformFlags & TransformFlags.ContainsSuper) {
2761+
transformFlags |= TransformFlags.ContainsLexicalThis;
2762+
}
27552763
}
27562764

27572765
if (expression.kind === SyntaxKind.ImportKeyword) {
@@ -2768,22 +2776,6 @@ namespace ts {
27682776
return transformFlags & ~TransformFlags.ArrayLiteralOrCallOrNewExcludes;
27692777
}
27702778

2771-
function isSuperOrSuperProperty(node: Node) {
2772-
node = skipOuterExpressions(node);
2773-
switch (node.kind) {
2774-
case SyntaxKind.SuperKeyword:
2775-
return true;
2776-
2777-
case SyntaxKind.PropertyAccessExpression:
2778-
case SyntaxKind.ElementAccessExpression:
2779-
const expression = (<PropertyAccessExpression | ElementAccessExpression>node).expression;
2780-
const expressionKind = expression.kind;
2781-
return expressionKind === SyntaxKind.SuperKeyword;
2782-
}
2783-
2784-
return false;
2785-
}
2786-
27872779
function computeNewExpression(node: NewExpression, subtreeFlags: TransformFlags) {
27882780
let transformFlags = subtreeFlags;
27892781
if (node.typeArguments) {
@@ -2878,7 +2870,7 @@ namespace ts {
28782870
}
28792871

28802872
node.transformFlags = transformFlags | TransformFlags.HasComputedFlags;
2881-
return transformFlags & ~TransformFlags.NodeExcludes;
2873+
return transformFlags & ~TransformFlags.OuterExpressionExcludes;
28822874
}
28832875

28842876
function computeClassDeclaration(node: ClassDeclaration, subtreeFlags: TransformFlags) {
@@ -3197,17 +3189,32 @@ namespace ts {
31973189

31983190
function computePropertyAccess(node: PropertyAccessExpression, subtreeFlags: TransformFlags) {
31993191
let transformFlags = subtreeFlags;
3200-
const expression = node.expression;
3201-
const expressionKind = expression.kind;
32023192

32033193
// If a PropertyAccessExpression starts with a super keyword, then it is
32043194
// ES6 syntax, and requires a lexical `this` binding.
3205-
if (expressionKind === SyntaxKind.SuperKeyword) {
3206-
transformFlags |= TransformFlags.ContainsLexicalThis;
3195+
if (transformFlags & TransformFlags.Super) {
3196+
transformFlags ^= TransformFlags.Super;
3197+
transformFlags |= TransformFlags.ContainsSuper;
32073198
}
32083199

32093200
node.transformFlags = transformFlags | TransformFlags.HasComputedFlags;
3210-
return transformFlags & ~TransformFlags.NodeExcludes;
3201+
return transformFlags & ~TransformFlags.PropertyAccessExcludes;
3202+
}
3203+
3204+
function computeElementAccess(node: ElementAccessExpression, subtreeFlags: TransformFlags) {
3205+
let transformFlags = subtreeFlags;
3206+
const expression = node.expression;
3207+
const expressionFlags = expression.transformFlags; // We do not want to aggregate flags from the argument expression for super/this capturing
3208+
3209+
// If an ElementAccessExpression starts with a super keyword, then it is
3210+
// ES6 syntax, and requires a lexical `this` binding.
3211+
if (expressionFlags & TransformFlags.Super) {
3212+
transformFlags &= ~TransformFlags.Super;
3213+
transformFlags |= TransformFlags.ContainsSuper;
3214+
}
3215+
3216+
node.transformFlags = transformFlags | TransformFlags.HasComputedFlags;
3217+
return transformFlags & ~TransformFlags.PropertyAccessExcludes;
32113218
}
32123219

32133220
function computeVariableDeclaration(node: VariableDeclaration, subtreeFlags: TransformFlags) {
@@ -3327,6 +3334,11 @@ namespace ts {
33273334
transformFlags |= TransformFlags.AssertESNext | TransformFlags.AssertES2017;
33283335
break;
33293336

3337+
case SyntaxKind.TypeAssertionExpression:
3338+
case SyntaxKind.AsExpression:
3339+
case SyntaxKind.PartiallyEmittedExpression:
3340+
excludeFlags = TransformFlags.OuterExpressionExcludes;
3341+
// fallthrough
33303342
case SyntaxKind.PublicKeyword:
33313343
case SyntaxKind.PrivateKeyword:
33323344
case SyntaxKind.ProtectedKeyword:
@@ -3335,8 +3347,6 @@ namespace ts {
33353347
case SyntaxKind.ConstKeyword:
33363348
case SyntaxKind.EnumDeclaration:
33373349
case SyntaxKind.EnumMember:
3338-
case SyntaxKind.TypeAssertionExpression:
3339-
case SyntaxKind.AsExpression:
33403350
case SyntaxKind.NonNullExpression:
33413351
case SyntaxKind.ReadonlyKeyword:
33423352
// These nodes are TypeScript syntax.
@@ -3464,7 +3474,8 @@ namespace ts {
34643474

34653475
case SyntaxKind.SuperKeyword:
34663476
// This node is ES6 syntax.
3467-
transformFlags |= TransformFlags.AssertES2015;
3477+
transformFlags |= TransformFlags.AssertES2015 | TransformFlags.Super;
3478+
excludeFlags = TransformFlags.OuterExpressionExcludes; // must be set to persist `Super`
34683479
break;
34693480

34703481
case SyntaxKind.ThisKeyword:
@@ -3621,6 +3632,15 @@ namespace ts {
36213632
case SyntaxKind.ObjectBindingPattern:
36223633
case SyntaxKind.ArrayBindingPattern:
36233634
return TransformFlags.BindingPatternExcludes;
3635+
case SyntaxKind.TypeAssertionExpression:
3636+
case SyntaxKind.AsExpression:
3637+
case SyntaxKind.PartiallyEmittedExpression:
3638+
case SyntaxKind.ParenthesizedExpression:
3639+
case SyntaxKind.SuperKeyword:
3640+
return TransformFlags.OuterExpressionExcludes;
3641+
case SyntaxKind.PropertyAccessExpression:
3642+
case SyntaxKind.ElementAccessExpression:
3643+
return TransformFlags.PropertyAccessExcludes;
36243644
default:
36253645
return TransformFlags.NodeExcludes;
36263646
}

‎src/compiler/types.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4448,6 +4448,8 @@ namespace ts {
44484448
ContainsYield = 1 << 24,
44494449
ContainsHoistedDeclarationOrCompletion = 1 << 25,
44504450
ContainsDynamicImport = 1 << 26,
4451+
Super = 1 << 27,
4452+
ContainsSuper = 1 << 28,
44514453

44524454
// Please leave this as 1 << 29.
44534455
// It is the maximum bit we can set before we outgrow the size of a v8 small integer (SMI) on an x86 system.
@@ -4468,7 +4470,9 @@ namespace ts {
44684470
// Scope Exclusions
44694471
// - Bitmasks that exclude flags from propagating out of a specific context
44704472
// into the subtree flags of their container.
4471-
NodeExcludes = TypeScript | ES2015 | DestructuringAssignment | Generator | HasComputedFlags,
4473+
OuterExpressionExcludes = TypeScript | ES2015 | DestructuringAssignment | Generator | HasComputedFlags,
4474+
PropertyAccessExcludes = OuterExpressionExcludes | Super,
4475+
NodeExcludes = PropertyAccessExcludes | ContainsSuper,
44724476
ArrowFunctionExcludes = NodeExcludes | ContainsDecorators | ContainsDefaultValueAssignments | ContainsLexicalThis | ContainsParameterPropertyAssignments | ContainsBlockScopedBinding | ContainsYield | ContainsHoistedDeclarationOrCompletion | ContainsBindingPattern | ContainsObjectRest,
44734477
FunctionExcludes = NodeExcludes | ContainsDecorators | ContainsDefaultValueAssignments | ContainsCapturedLexicalThis | ContainsLexicalThis | ContainsParameterPropertyAssignments | ContainsBlockScopedBinding | ContainsYield | ContainsHoistedDeclarationOrCompletion | ContainsBindingPattern | ContainsObjectRest,
44744478
ConstructorExcludes = NodeExcludes | ContainsDefaultValueAssignments | ContainsLexicalThis | ContainsCapturedLexicalThis | ContainsBlockScopedBinding | ContainsYield | ContainsHoistedDeclarationOrCompletion | ContainsBindingPattern | ContainsObjectRest,

‎tests/baselines/reference/decoratorOnClassMethod12.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ var __decorate = (this && this.__decorate) || function (decorators, target, key,
2828
};
2929
var M;
3030
(function (M) {
31-
var _this = this;
3231
var S = /** @class */ (function () {
3332
function S() {
3433
}

‎tests/baselines/reference/superAccess2.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ var __extends = (this && this.__extends) || (function () {
3535
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
3636
};
3737
})();
38-
var _this = this;
3938
var P = /** @class */ (function () {
4039
function P() {
4140
}

‎tests/baselines/reference/superElementAccess.errors.txt

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
tests/cases/compiler/superElementAccess.ts(7,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
2+
tests/cases/compiler/superElementAccess.ts(8,9): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
3+
4+
5+
==== tests/cases/compiler/superElementAccess.ts (2 errors) ====
6+
class MyBase {
7+
m1(a: string) { return a; }
8+
private p1() { }
9+
m2: () => void = function () { }
10+
d1: number = 42;
11+
private d2: number = 42;
12+
get value() {return 0 }
13+
~~~~~
14+
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
15+
set value(v: number) { }
16+
~~~~~
17+
!!! error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.
18+
}
19+
20+
21+
class MyDerived extends MyBase {
22+
23+
foo() {
24+
super["m1"]("hi"); // Should be allowed, method on base prototype
25+
26+
var l2 = super["m1"].bind(this); // Should be allowed, can access properties as well as invoke
27+
28+
var x: (a: string) => string = super["m1"]; // Should be allowed, can assign to var with compatible signature
29+
30+
super["m2"].bind(this); // Should error, instance property, not a public instance member function
31+
32+
super["p1"](); // Should error, private not public instance member function
33+
34+
var l1 = super["d1"]; // Should error, instance data property not a public instance member function
35+
36+
var l1 = super["d2"]; // Should error, instance data property not a public instance member function
37+
38+
super["m1"] = function (a: string) { return ""; }; // Should be allowed, we will not restrict assignment
39+
40+
super["value"] = 0; // Should error, instance data property not a public instance member function
41+
42+
var z = super["value"]; // Should error, instance data property not a public instance member function
43+
}
44+
}

‎tests/baselines/reference/superElementAccess.js

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
//// [superElementAccess.ts]
2+
class MyBase {
3+
m1(a: string) { return a; }
4+
private p1() { }
5+
m2: () => void = function () { }
6+
d1: number = 42;
7+
private d2: number = 42;
8+
get value() {return 0 }
9+
set value(v: number) { }
10+
}
11+
12+
13+
class MyDerived extends MyBase {
14+
15+
foo() {
16+
super["m1"]("hi"); // Should be allowed, method on base prototype
17+
18+
var l2 = super["m1"].bind(this); // Should be allowed, can access properties as well as invoke
19+
20+
var x: (a: string) => string = super["m1"]; // Should be allowed, can assign to var with compatible signature
21+
22+
super["m2"].bind(this); // Should error, instance property, not a public instance member function
23+
24+
super["p1"](); // Should error, private not public instance member function
25+
26+
var l1 = super["d1"]; // Should error, instance data property not a public instance member function
27+
28+
var l1 = super["d2"]; // Should error, instance data property not a public instance member function
29+
30+
super["m1"] = function (a: string) { return ""; }; // Should be allowed, we will not restrict assignment
31+
32+
super["value"] = 0; // Should error, instance data property not a public instance member function
33+
34+
var z = super["value"]; // Should error, instance data property not a public instance member function
35+
}
36+
}
37+
38+
//// [superElementAccess.js]
39+
var __extends = (this && this.__extends) || (function () {
40+
var extendStatics = Object.setPrototypeOf ||
41+
({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
42+
function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; };
43+
return function (d, b) {
44+
extendStatics(d, b);
45+
function __() { this.constructor = d; }
46+
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
47+
};
48+
})();
49+
var MyBase = /** @class */ (function () {
50+
function MyBase() {
51+
this.m2 = function () { };
52+
this.d1 = 42;
53+
this.d2 = 42;
54+
}
55+
MyBase.prototype.m1 = function (a) { return a; };
56+
MyBase.prototype.p1 = function () { };
57+
Object.defineProperty(MyBase.prototype, "value", {
58+
get: function () { return 0; },
59+
set: function (v) { },
60+
enumerable: true,
61+
configurable: true
62+
});
63+
return MyBase;
64+
}());
65+
var MyDerived = /** @class */ (function (_super) {
66+
__extends(MyDerived, _super);
67+
function MyDerived() {
68+
return _super !== null && _super.apply(this, arguments) || this;
69+
}
70+
MyDerived.prototype.foo = function () {
71+
_super.prototype["m1"].call(this, "hi"); // Should be allowed, method on base prototype
72+
var l2 = (_a = _super.prototype["m1"]).bind.call(_a, this); // Should be allowed, can access properties as well as invoke
73+
var x = _super.prototype["m1"]; // Should be allowed, can assign to var with compatible signature
74+
(_b = _super.prototype["m2"]).bind.call(_b, this); // Should error, instance property, not a public instance member function
75+
_super.prototype["p1"].call(this); // Should error, private not public instance member function
76+
var l1 = _super.prototype["d1"]; // Should error, instance data property not a public instance member function
77+
var l1 = _super.prototype["d2"]; // Should error, instance data property not a public instance member function
78+
_super.prototype["m1"] = function (a) { return ""; }; // Should be allowed, we will not restrict assignment
79+
_super.prototype["value"] = 0; // Should error, instance data property not a public instance member function
80+
var z = _super.prototype["value"]; // Should error, instance data property not a public instance member function
81+
var _a, _b;
82+
};
83+
return MyDerived;
84+
}(MyBase));

0 commit comments

Comments
 (0)