Skip to content

Fix handling of aruments in the emitter #38880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 13, 2020
Merged

Fix handling of aruments in the emitter #38880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
185e7ad
Fix handling of `aruments` in the emitter
elibarzilay Jun 1, 2020
32eece8
Simplify `visitObjectLiteralExpression`
elibarzilay Jun 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36071,15 +36071,16 @@ namespace ts {
// Emitter support

function isArgumentsLocalBinding(nodeIn: Identifier): boolean {
if (!isGeneratedIdentifier(nodeIn)) {
const node = getParseTreeNode(nodeIn, isIdentifier);
if (node) {
const isPropertyName = node.parent.kind === SyntaxKind.PropertyAccessExpression && (<PropertyAccessExpression>node.parent).name === node;
return !isPropertyName && getReferencedValueSymbol(node) === argumentsSymbol;
}
}

return false;
// Note: does not handle isShorthandPropertyAssignment (and probably a few more)
if (isGeneratedIdentifier(nodeIn)) return false;
const node = getParseTreeNode(nodeIn, isIdentifier);
if (!node) return false;
const parent = node.parent;
if (!parent) return false;
const isPropertyName = ((isPropertyAccessExpression(parent)
|| isPropertyAssignment(parent))
&& parent.name === node);
return !isPropertyName && getReferencedValueSymbol(node) === argumentsSymbol;
}

function moduleExportsSomeValue(moduleReferenceExpression: Expression): boolean {
Expand Down
83 changes: 36 additions & 47 deletions src/compiler/transformers/es2015.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,13 +583,10 @@ namespace ts {
if (!convertedLoopState) {
return node;
}
if (isGeneratedIdentifier(node)) {
return node;
}
if (node.escapedText !== "arguments" || !resolver.isArgumentsLocalBinding(node)) {
return node;
if (resolver.isArgumentsLocalBinding(node)) {
return convertedLoopState.argumentsName || (convertedLoopState.argumentsName = createUniqueName("arguments"));
}
return convertedLoopState.argumentsName || (convertedLoopState.argumentsName = createUniqueName("arguments"));
return node;
}

function visitBreakOrContinueStatement(node: BreakOrContinueStatement): Statement {
Expand Down Expand Up @@ -2569,62 +2566,54 @@ namespace ts {
* @param node An ObjectLiteralExpression node.
*/
function visitObjectLiteralExpression(node: ObjectLiteralExpression): Expression {
// We are here because a ComputedPropertyName was used somewhere in the expression.
const properties = node.properties;
const numProperties = properties.length;

// Find the first computed property.
// Everything until that point can be emitted as part of the initial object literal.
let numInitialProperties = numProperties;
let numInitialPropertiesWithoutYield = numProperties;
for (let i = 0; i < numProperties; i++) {
let numInitialProperties = -1, hasComputed = false;
for (let i = 0; i < properties.length; i++) {
const property = properties[i];
if ((property.transformFlags & TransformFlags.ContainsYield && hierarchyFacts & HierarchyFacts.AsyncFunctionBody)
&& i < numInitialPropertiesWithoutYield) {
numInitialPropertiesWithoutYield = i;
}
if (Debug.checkDefined(property.name).kind === SyntaxKind.ComputedPropertyName) {
if ((property.transformFlags & TransformFlags.ContainsYield &&
hierarchyFacts & HierarchyFacts.AsyncFunctionBody)
|| (hasComputed = Debug.checkDefined(property.name).kind === SyntaxKind.ComputedPropertyName)) {
numInitialProperties = i;
break;
}
}

if (numInitialProperties !== numProperties) {
if (numInitialPropertiesWithoutYield < numInitialProperties) {
numInitialProperties = numInitialPropertiesWithoutYield;
}
if (numInitialProperties < 0) {
return visitEachChild(node, visitor, context);
}

// For computed properties, we need to create a unique handle to the object
// literal so we can modify it without risking internal assignments tainting the object.
const temp = createTempVariable(hoistVariableDeclaration);
// For computed properties, we need to create a unique handle to the object
// literal so we can modify it without risking internal assignments tainting the object.
const temp = createTempVariable(hoistVariableDeclaration);

// Write out the first non-computed properties, then emit the rest through indexing on the temp variable.
const expressions: Expression[] = [];
const assignment = createAssignment(
temp,
setEmitFlags(
createObjectLiteral(
visitNodes(properties, visitor, isObjectLiteralElementLike, 0, numInitialProperties),
node.multiLine
),
EmitFlags.Indented
)
);
// Write out the first non-computed properties, then emit the rest through indexing on the temp variable.
const expressions: Expression[] = [];
const assignment = createAssignment(
temp,
setEmitFlags(
createObjectLiteral(
visitNodes(properties, visitor, isObjectLiteralElementLike, 0, numInitialProperties),
node.multiLine
),
hasComputed ? EmitFlags.Indented : 0
)
);

if (node.multiLine) {
startOnNewLine(assignment);
}
if (node.multiLine) {
startOnNewLine(assignment);
}

expressions.push(assignment);
expressions.push(assignment);

addObjectLiteralMembers(expressions, node, temp, numInitialProperties);
addObjectLiteralMembers(expressions, node, temp, numInitialProperties);

// We need to clone the temporary identifier so that we can write it on a
// new line
expressions.push(node.multiLine ? startOnNewLine(getMutableClone(temp)) : temp);
return inlineExpressions(expressions);
}
return visitEachChild(node, visitor, context);
// We need to clone the temporary identifier so that we can write it on a
// new line
expressions.push(node.multiLine ? startOnNewLine(getMutableClone(temp)) : temp);
return inlineExpressions(expressions);
}

interface ForStatementWithConvertibleInitializer extends ForStatement {
Expand Down Expand Up @@ -3517,7 +3506,7 @@ namespace ts {
return setTextRange(
createPropertyAssignment(
node.name,
getSynthesizedClone(node.name)
visitIdentifier(getSynthesizedClone(node.name))
),
/*location*/ node
);
Expand Down
29 changes: 29 additions & 0 deletions tests/baselines/reference/argumentsAsPropertyName2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//// [argumentsAsPropertyName2.ts]
// target: es5

function foo() {
for (let x = 0; x < 1; ++x) {
let i : number;
[].forEach(function () { i });
({ arguments: 0 });
({ arguments });
({ arguments: arguments });
}
}


//// [argumentsAsPropertyName2.js]
// target: es5
function foo() {
var _loop_1 = function (x) {
var i;
[].forEach(function () { i; });
({ arguments: 0 });
({ arguments: arguments_1 });
({ arguments: arguments_1 });
};
var arguments_1 = arguments;
for (var x = 0; x < 1; ++x) {
_loop_1(x);
}
}
31 changes: 31 additions & 0 deletions tests/baselines/reference/argumentsAsPropertyName2.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
=== tests/cases/compiler/argumentsAsPropertyName2.ts ===
// target: es5

function foo() {
>foo : Symbol(foo, Decl(argumentsAsPropertyName2.ts, 0, 0))

for (let x = 0; x < 1; ++x) {
>x : Symbol(x, Decl(argumentsAsPropertyName2.ts, 3, 12))
>x : Symbol(x, Decl(argumentsAsPropertyName2.ts, 3, 12))
>x : Symbol(x, Decl(argumentsAsPropertyName2.ts, 3, 12))

let i : number;
>i : Symbol(i, Decl(argumentsAsPropertyName2.ts, 4, 11))

[].forEach(function () { i });
>[].forEach : Symbol(Array.forEach, Decl(lib.es5.d.ts, --, --))
>forEach : Symbol(Array.forEach, Decl(lib.es5.d.ts, --, --))
>i : Symbol(i, Decl(argumentsAsPropertyName2.ts, 4, 11))

({ arguments: 0 });
>arguments : Symbol(arguments, Decl(argumentsAsPropertyName2.ts, 6, 10))

({ arguments });
>arguments : Symbol(arguments, Decl(argumentsAsPropertyName2.ts, 7, 10))

({ arguments: arguments });
>arguments : Symbol(arguments, Decl(argumentsAsPropertyName2.ts, 8, 10))
>arguments : Symbol(arguments)
}
}

45 changes: 45 additions & 0 deletions tests/baselines/reference/argumentsAsPropertyName2.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
=== tests/cases/compiler/argumentsAsPropertyName2.ts ===
// target: es5

function foo() {
>foo : () => void

for (let x = 0; x < 1; ++x) {
>x : number
>0 : 0
>x < 1 : boolean
>x : number
>1 : 1
>++x : number
>x : number

let i : number;
>i : number

[].forEach(function () { i });
>[].forEach(function () { i }) : void
>[].forEach : (callbackfn: (value: any, index: number, array: any[]) => void, thisArg?: any) => void
>[] : undefined[]
>forEach : (callbackfn: (value: any, index: number, array: any[]) => void, thisArg?: any) => void
>function () { i } : () => void
>i : number

({ arguments: 0 });
>({ arguments: 0 }) : { arguments: number; }
>{ arguments: 0 } : { arguments: number; }
>arguments : number
>0 : 0

({ arguments });
>({ arguments }) : { arguments: IArguments; }
>{ arguments } : { arguments: IArguments; }
>arguments : IArguments

({ arguments: arguments });
>({ arguments: arguments }) : { arguments: IArguments; }
>{ arguments: arguments } : { arguments: IArguments; }
>arguments : IArguments
>arguments : IArguments
}
}

8 changes: 4 additions & 4 deletions tests/baselines/reference/es5-asyncFunctionObjectLiterals.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ function objectLiteral5() {
switch (_b.label) {
case 0:
_a = {
a: y
};
a: y
};
return [4 /*yield*/, b];
case 1:
x = (_a[_b.sent()] = z,
Expand All @@ -165,8 +165,8 @@ function objectLiteral6() {
switch (_c.label) {
case 0:
_b = {
a: y
};
a: y
};
_a = b;
return [4 /*yield*/, z];
case 1:
Expand Down
Loading