Skip to content

Commit f46cb89

Browse files
CyrusNajmabadiCyrusNajmabadi
committed
Be more conservative on when we'll emit a function in a single line.
1 parent 8d0dc91 commit f46cb89

File tree

52 files changed

+428
-210
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+428
-210
lines changed

‎src/compiler/emitter.ts

Lines changed: 20 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -3932,113 +3932,43 @@ module ts {
39323932
}
39333933

39343934
function emitBlockFunctionBody(node: FunctionLikeDeclaration, body: Block) {
3935-
// If the body has no statements, and we know there's no code that would have to
3936-
// run that could cause side effects, then just do a simple emit if the empty
3937-
// block.
3938-
if (body.statements.length === 0 && !hasPossibleSideEffectingParameterInitializers(node)) {
3935+
// If the body has no statements, and we know there's no code that would cause any
3936+
// prologue to be emitted, then just do a simple emit if the empty block.
3937+
if (body.statements.length === 0 && !anyParameterHasBindingPatternOrInitializer(node)) {
39393938
emitFunctionBodyWithNoStatements(node, body);
39403939
}
39413940
else {
39423941
emitFunctionBodyWithStatements(node, body);
39433942
}
39443943
}
39453944

3946-
function hasPossibleSideEffectingParameterInitializers(func: FunctionLikeDeclaration) {
3947-
return forEach(func.parameters, hasPossibleSideEffects);
3945+
function anyParameterHasBindingPatternOrInitializer(func: FunctionLikeDeclaration) {
3946+
return forEach(func.parameters, hasBindingPatternOrInitializer);
39483947
}
39493948

3950-
function hasPossibleSideEffects(node: Node): boolean {
3951-
if (!node) {
3952-
return false;
3953-
}
3954-
3955-
switch (node.kind) {
3956-
// TODO(cyrusn): Increase the number of cases we support for determining if
3957-
// something is side effect free.
3958-
//
3959-
// NOTE(cyrusn): Some expressions may seem to be side effect free, but may
3960-
// actually have side effects. For example, a binary + expression may cause
3961-
// the toString method to be called on value, which may have side effects.
3962-
3963-
// These are the set of syntactic productions which we know could definitely
3964-
// have side effects:
3965-
case SyntaxKind.CallExpression:
3966-
case SyntaxKind.NewExpression:
3967-
return true;
3968-
3969-
case SyntaxKind.PropertyAccessExpression:
3970-
case SyntaxKind.ElementAccessExpression:
3971-
// Property/Element access might end up causing an accessor to run. As
3972-
// such, we have to assume there will be side effects.
3973-
return true;
3974-
3975-
// These are the set of syntactic productions which we know definitely do not
3976-
// have side effects:
3977-
case SyntaxKind.StringLiteral:
3978-
case SyntaxKind.NoSubstitutionTemplateLiteral:
3979-
case SyntaxKind.NumericLiteral:
3980-
case SyntaxKind.RegularExpressionLiteral:
3981-
case SyntaxKind.TrueKeyword:
3982-
case SyntaxKind.FalseKeyword:
3983-
case SyntaxKind.NullKeyword:
3984-
case SyntaxKind.Identifier:
3985-
case SyntaxKind.FunctionExpression:
3986-
case SyntaxKind.ArrowFunction:
3987-
case SyntaxKind.ShorthandPropertyAssignment:
3988-
case SyntaxKind.MethodDeclaration:
3989-
case SyntaxKind.GetAccessor:
3990-
case SyntaxKind.SetAccessor:
3991-
return false;
3992-
3993-
// These constructs may or may not have side effects depending on their
3994-
// constituent children.
3995-
case SyntaxKind.ArrayBindingPattern:
3996-
case SyntaxKind.ObjectBindingPattern:
3997-
return forEach((<BindingPattern>node).elements, hasPossibleSideEffects);
3998-
3999-
case SyntaxKind.BindingElement:
4000-
return hasPossibleSideEffects((<BindingElement>node).name) ||
4001-
hasPossibleSideEffects((<BindingElement>node).initializer);
4002-
4003-
case SyntaxKind.Parameter:
4004-
return hasPossibleSideEffects((<ParameterDeclaration>node).name) ||
4005-
hasPossibleSideEffects((<ParameterDeclaration>node).initializer);
4006-
4007-
case SyntaxKind.ArrayLiteralExpression:
4008-
return forEach((<ArrayLiteralExpression>node).elements, hasPossibleSideEffects);
4009-
4010-
case SyntaxKind.ParenthesizedExpression:
4011-
return hasPossibleSideEffects((<ParenthesizedExpression>node).expression);
4012-
4013-
case SyntaxKind.ObjectLiteralExpression:
4014-
return forEach((<ObjectLiteralExpression>node).properties, hasPossibleSideEffects);
4015-
4016-
case SyntaxKind.PropertyAssignment:
4017-
return hasPossibleSideEffects((<PropertyAssignment>node).name) ||
4018-
hasPossibleSideEffects((<PropertyAssignment>node).initializer);
4019-
4020-
case SyntaxKind.ComputedPropertyName:
4021-
return hasPossibleSideEffects((<ComputedPropertyName>node).expression);
4022-
4023-
default:
4024-
// We are conservative. Unless we have proved something is side effect
4025-
// free, we assume it has possible side effects.
4026-
return true;
4027-
}
3949+
function hasBindingPatternOrInitializer(parameter: ParameterDeclaration) {
3950+
return parameter.initializer || isBindingPattern(parameter.name);
40283951
}
40293952

40303953
function emitFunctionBodyWithNoStatements(node: FunctionLikeDeclaration, body: Block) {
4031-
if (isSingleLineEmptyBlock(node.body)) {
4032-
write(" { }");
3954+
var singleLine = isSingleLineEmptyBlock(node.body);
3955+
3956+
write(" {");
3957+
if (singleLine) {
3958+
write(" ");
40333959
}
40343960
else {
4035-
write(" {");
4036-
writeLine();
40373961
increaseIndent();
4038-
emitLeadingCommentsOfPosition(body.statements.end);
3962+
writeLine();
3963+
}
3964+
3965+
emitLeadingCommentsOfPosition(body.statements.end);
3966+
3967+
if (!singleLine) {
40393968
decreaseIndent();
4040-
emitToken(SyntaxKind.CloseBraceToken, body.statements.end);
40413969
}
3970+
3971+
emitToken(SyntaxKind.CloseBraceToken, body.statements.end);
40423972
}
40433973

40443974
function emitFunctionBodyWithStatements(node: FunctionLikeDeclaration, body: Block) {

‎tests/baselines/reference/FunctionDeclaration10_es6.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@ function * foo(a = yield => yield) {
44

55
//// [FunctionDeclaration10_es6.js]
66
function foo(a) {
7+
if (a === void 0) { a = function (yield) { return yield; }; }
78
}

‎tests/baselines/reference/FunctionDeclaration3_es6.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@ function f(yield = yield) {
44

55
//// [FunctionDeclaration3_es6.js]
66
function f(yield) {
7+
if (yield === void 0) { yield = yield; }
78
}

‎tests/baselines/reference/FunctionDeclaration6_es6.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@ function*foo(a = yield) {
44

55
//// [FunctionDeclaration6_es6.js]
66
function foo(a) {
7+
if (a === void 0) { a = yield; }
78
}

‎tests/baselines/reference/FunctionDeclaration7_es6.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@ function*bar() {
99
function bar() {
1010
// 'yield' here is an identifier, and not a yield expression.
1111
function foo(a) {
12+
if (a === void 0) { a = yield; }
1213
}
1314
}

‎tests/baselines/reference/accessorWithInitializer.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,16 @@ var C = (function () {
1010
function C() {
1111
}
1212
Object.defineProperty(C.prototype, "X", {
13-
set: function (v) { },
13+
set: function (v) {
14+
if (v === void 0) { v = 0; }
15+
},
1416
enumerable: true,
1517
configurable: true
1618
});
1719
Object.defineProperty(C, "X", {
18-
set: function (v2) { },
20+
set: function (v2) {
21+
if (v2 === void 0) { v2 = 0; }
22+
},
1923
enumerable: true,
2024
configurable: true
2125
});

‎tests/baselines/reference/callSignatureWithOptionalParameterAndInitializer.js

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,15 @@ b.b(1);
5757

5858
//// [callSignatureWithOptionalParameterAndInitializer.js]
5959
// Optional parameters cannot also have initializer expressions, these are all errors
60-
function foo(x) { }
61-
var f = function foo(x) { };
62-
var f2 = function (x, y) { };
60+
function foo(x) {
61+
if (x === void 0) { x = 1; }
62+
}
63+
var f = function foo(x) {
64+
if (x === void 0) { x = 1; }
65+
};
66+
var f2 = function (x, y) {
67+
if (y === void 0) { y = 1; }
68+
};
6369
foo(1);
6470
foo();
6571
f(1);
@@ -69,7 +75,9 @@ f2(1, 2);
6975
var C = (function () {
7076
function C() {
7177
}
72-
C.prototype.foo = function (x) { };
78+
C.prototype.foo = function (x) {
79+
if (x === void 0) { x = 1; }
80+
};
7381
return C;
7482
})();
7583
var c;
@@ -86,9 +94,15 @@ a(1);
8694
a.foo();
8795
a.foo(1);
8896
var b = {
89-
foo: function (x) { },
90-
a: function foo(x, y) { },
91-
b: function (x) { }
97+
foo: function (x) {
98+
if (x === void 0) { x = 1; }
99+
},
100+
a: function foo(x, y) {
101+
if (y === void 0) { y = ''; }
102+
},
103+
b: function (x) {
104+
if (x === void 0) { x = ''; }
105+
}
92106
};
93107
b.foo();
94108
b.foo(1);

‎tests/baselines/reference/callSignaturesWithParameterInitializers.js

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,15 @@ b.b(1);
5959

6060
//// [callSignaturesWithParameterInitializers.js]
6161
// Optional parameters allow initializers only in implementation signatures
62-
function foo(x) { }
63-
var f = function foo(x) { };
64-
var f2 = function (x, y) { };
62+
function foo(x) {
63+
if (x === void 0) { x = 1; }
64+
}
65+
var f = function foo(x) {
66+
if (x === void 0) { x = 1; }
67+
};
68+
var f2 = function (x, y) {
69+
if (y === void 0) { y = 1; }
70+
};
6571
foo(1);
6672
foo();
6773
f(1);
@@ -71,7 +77,9 @@ f2(1, 2);
7177
var C = (function () {
7278
function C() {
7379
}
74-
C.prototype.foo = function (x) { };
80+
C.prototype.foo = function (x) {
81+
if (x === void 0) { x = 1; }
82+
};
7583
return C;
7684
})();
7785
var c;
@@ -89,9 +97,15 @@ a(1);
8997
a.foo();
9098
a.foo(1);
9199
var b = {
92-
foo: function (x) { },
93-
a: function foo(x, y) { },
94-
b: function (x) { }
100+
foo: function (x) {
101+
if (x === void 0) { x = 1; }
102+
},
103+
a: function foo(x, y) {
104+
if (y === void 0) { y = 1; }
105+
},
106+
b: function (x) {
107+
if (x === void 0) { x = 1; }
108+
}
95109
};
96110
b.foo();
97111
b.foo(1);

‎tests/baselines/reference/callSignaturesWithParameterInitializers2.js

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,21 +28,29 @@ b.foo(1);
2828
//// [callSignaturesWithParameterInitializers2.js]
2929
// Optional parameters allow initializers only in implementation signatures
3030
// All the below declarations are errors
31-
function foo(x) { }
31+
function foo(x) {
32+
if (x === void 0) { x = 1; }
33+
}
3234
foo(1);
3335
foo();
3436
var C = (function () {
3537
function C() {
3638
}
37-
C.prototype.foo = function (x) { };
39+
C.prototype.foo = function (x) {
40+
if (x === void 0) { x = 1; }
41+
};
3842
return C;
3943
})();
4044
var c;
4145
c.foo();
4246
c.foo(1);
4347
var b = {
44-
foo: function (x) { },
45-
foo: function (x) { }
48+
foo: function (x) {
49+
if (x === void 0) { x = 1; }
50+
},
51+
foo: function (x) {
52+
if (x === void 0) { x = 1; }
53+
}
4654
};
4755
b.foo();
4856
b.foo(1);

‎tests/baselines/reference/collisionCodeGenModuleWithFunctionChildren.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ module M {
2323
var M;
2424
(function (_M) {
2525
_M.x = 3;
26-
function fn(M, p) { }
26+
function fn(M, p) {
27+
if (p === void 0) { p = _M.x; }
28+
}
2729
})(M || (M = {}));
2830
var M;
2931
(function (_M) {

0 commit comments

Comments
 (0)