Skip to content

Commit f48889f

Browse files
joeywattsjoeywatts
committed
Fix property name bindings for class expr in loops
1 parent cf24c4f commit f48889f

23 files changed

+411
-36
lines changed

‎src/compiler/checker.ts

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20722,6 +20722,10 @@ namespace ts {
2072220722
return findAncestor(node, n => n === container ? "quit" : n === container.initializer || n === container.condition || n === container.incrementor || n === container.statement);
2072320723
}
2072420724

20725+
function getEnclosingIterationStatement(node: Node): Node | undefined {
20726+
return findAncestor(node, n => (!n || nodeStartsNewLexicalEnvironment(n)) ? "quit" : isIterationStatement(n, /*lookInLabeledStatements*/ false));
20727+
}
20728+
2072520729
function checkNestedBlockScopedBinding(node: Identifier, symbol: Symbol): void {
2072620730
if (languageVersion >= ScriptTarget.ES2015 ||
2072720731
(symbol.flags & (SymbolFlags.BlockScopedVariable | SymbolFlags.Class)) === 0 ||
@@ -20737,18 +20741,9 @@ namespace ts {
2073720741

2073820742
const container = getEnclosingBlockScopeContainer(symbol.valueDeclaration);
2073920743
const usedInFunction = isInsideFunction(node.parent, container);
20740-
let current = container;
20741-
20742-
let containedInIterationStatement = false;
20743-
while (current && !nodeStartsNewLexicalEnvironment(current)) {
20744-
if (isIterationStatement(current, /*lookInLabeledStatements*/ false)) {
20745-
containedInIterationStatement = true;
20746-
break;
20747-
}
20748-
current = current.parent;
20749-
}
2075020744

20751-
if (containedInIterationStatement) {
20745+
const enclosingIterationStatement = getEnclosingIterationStatement(container);
20746+
if (enclosingIterationStatement) {
2075220747
if (usedInFunction) {
2075320748
// mark iteration statement as containing block-scoped binding captured in some function
2075420749
let capturesBlockScopeBindingInLoopBody = true;
@@ -20770,7 +20765,7 @@ namespace ts {
2077020765
}
2077120766
}
2077220767
if (capturesBlockScopeBindingInLoopBody) {
20773-
getNodeLinks(current).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
20768+
getNodeLinks(enclosingIterationStatement).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
2077420769
}
2077520770
}
2077620771

@@ -22369,6 +22364,20 @@ namespace ts {
2236922364
const links = getNodeLinks(node.expression);
2237022365
if (!links.resolvedType) {
2237122366
links.resolvedType = checkExpression(node.expression);
22367+
// The computed property name of a non-static class field within a loop must be stored in a block-scoped binding.
22368+
// (It needs to be bound at class evaluation time.)
22369+
if (isPropertyDeclaration(node.parent) && !hasStaticModifier(node.parent) && isClassExpression(node.parent.parent)) {
22370+
const container = getEnclosingBlockScopeContainer(node.parent.parent);
22371+
const enclosingIterationStatement = getEnclosingIterationStatement(container);
22372+
if (enclosingIterationStatement) {
22373+
// The computed field name will use a block scoped binding which can be unique for each iteration of the loop.
22374+
getNodeLinks(enclosingIterationStatement).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
22375+
// The generated variable which stores the computed field name must be block-scoped.
22376+
getNodeLinks(node).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
22377+
// The generated variable which stores the class must be block-scoped.
22378+
getNodeLinks(node.parent.parent).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
22379+
}
22380+
}
2237222381
// This will allow types number, string, symbol or any. It will also allow enums, the unknown
2237322382
// type, and any union of these types (like string | number).
2237422383
if (links.resolvedType.flags & TypeFlags.Nullable ||
@@ -28916,6 +28925,16 @@ namespace ts {
2891628925
for (let lexicalScope = getEnclosingBlockScopeContainer(node); !!lexicalScope; lexicalScope = getEnclosingBlockScopeContainer(lexicalScope)) {
2891728926
getNodeLinks(lexicalScope).flags |= NodeCheckFlags.ContainsClassWithPrivateIdentifiers;
2891828927
}
28928+
28929+
// If this is a private field in a class expression inside the body of a loop,
28930+
// then we must use a block-scoped binding to store the WeakMap.
28931+
if (isClassExpression(node.parent)) {
28932+
const enclosingIterationStatement = getEnclosingIterationStatement(node.parent);
28933+
if (enclosingIterationStatement) {
28934+
getNodeLinks(node.name).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
28935+
getNodeLinks(enclosingIterationStatement).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
28936+
}
28937+
}
2891928938
}
2892028939
}
2892128940

‎src/compiler/factory.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ namespace ts {
1818
resumeLexicalEnvironment: noop,
1919
startLexicalEnvironment: noop,
2020
suspendLexicalEnvironment: noop,
21+
startBlockScope: noop,
22+
endBlockScope: returnUndefined,
23+
addBlockScopedVariable: noop,
2124
addDiagnostic: noop,
2225
};
2326

‎src/compiler/transformer.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,9 @@ namespace ts {
152152
let lexicalEnvironmentFunctionDeclarationsStack: FunctionDeclaration[][] = [];
153153
let lexicalEnvironmentStackOffset = 0;
154154
let lexicalEnvironmentSuspended = false;
155+
let blockScopedVariableDeclarationsStack: Identifier[][] = [];
156+
let blockScopeStackOffset = 0;
157+
let blockScopedVariableDeclarations: Identifier[];
155158
let emitHelpers: EmitHelper[] | undefined;
156159
let onSubstituteNode: TransformationContext["onSubstituteNode"] = noEmitSubstitution;
157160
let onEmitNode: TransformationContext["onEmitNode"] = noEmitNotification;
@@ -170,6 +173,9 @@ namespace ts {
170173
endLexicalEnvironment,
171174
hoistVariableDeclaration,
172175
hoistFunctionDeclaration,
176+
startBlockScope,
177+
endBlockScope,
178+
addBlockScopedVariable,
173179
requestEmitHelper,
174180
readEmitHelpers,
175181
enableSubstitution,
@@ -408,6 +414,46 @@ namespace ts {
408414
return statements;
409415
}
410416

417+
/**
418+
* Starts a block scope. Any existing block hoisted variables are pushed onto the stack and the related storage variables are reset.
419+
*/
420+
function startBlockScope() {
421+
Debug.assert(state > TransformationState.Uninitialized, "Cannot start a block scope during initialization.");
422+
Debug.assert(state < TransformationState.Completed, "Cannot start a block scope after transformation has completed.");
423+
blockScopedVariableDeclarationsStack[blockScopeStackOffset] = blockScopedVariableDeclarations;
424+
blockScopeStackOffset++;
425+
blockScopedVariableDeclarations = undefined!;
426+
}
427+
428+
/**
429+
* Ends a block scope. The previous set of block hoisted variables are restored. Any hoisted declarations are returned.
430+
*/
431+
function endBlockScope() {
432+
Debug.assert(state > TransformationState.Uninitialized, "Cannot end a block scope during initialization.");
433+
Debug.assert(state < TransformationState.Completed, "Cannot end a block scope after transformation has completed.");
434+
const statements: Statement[] | undefined = some(blockScopedVariableDeclarations) ?
435+
[
436+
createVariableStatement(
437+
/*modifiers*/ undefined,
438+
createVariableDeclarationList(
439+
blockScopedVariableDeclarations.map(identifier => createVariableDeclaration(identifier)),
440+
NodeFlags.Let
441+
)
442+
)
443+
] : undefined;
444+
blockScopeStackOffset--;
445+
blockScopedVariableDeclarations = blockScopedVariableDeclarationsStack[blockScopeStackOffset];
446+
if (blockScopeStackOffset === 0) {
447+
blockScopedVariableDeclarationsStack = [];
448+
}
449+
return statements;
450+
}
451+
452+
function addBlockScopedVariable(name: Identifier): void {
453+
Debug.assert(blockScopeStackOffset > 0, "Cannot add a block scoped variable outside of an iteration body.");
454+
(blockScopedVariableDeclarations || (blockScopedVariableDeclarations = [])).push(name);
455+
}
456+
411457
function requestEmitHelper(helper: EmitHelper): void {
412458
Debug.assert(state > TransformationState.Uninitialized, "Cannot modify the transformation context during initialization.");
413459
Debug.assert(state < TransformationState.Completed, "Cannot modify the transformation context after transformation has completed.");

‎src/compiler/transformers/classFields.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ namespace ts {
3535
const {
3636
hoistVariableDeclaration,
3737
endLexicalEnvironment,
38-
resumeLexicalEnvironment
38+
resumeLexicalEnvironment,
39+
addBlockScopedVariable
3940
} = context;
4041
const resolver = context.getEmitResolver();
4142
const compilerOptions = context.getCompilerOptions();
@@ -312,7 +313,7 @@ namespace ts {
312313
visitNode(node.initializer, visitor, isForInitializer),
313314
visitNode(node.condition, visitor, isExpression),
314315
visitPostfixUnaryExpression(node.incrementor, /*valueIsDiscarded*/ true),
315-
visitNode(node.statement, visitor, isStatement)
316+
visitIterationBody(node.statement, visitor, context)
316317
);
317318
}
318319
return visitEachChild(node, visitor, context);
@@ -532,8 +533,10 @@ namespace ts {
532533
}
533534
else {
534535
const expressions: Expression[] = [];
535-
const isClassWithConstructorReference = resolver.getNodeCheckFlags(node) & NodeCheckFlags.ClassWithConstructorReference;
536-
const temp = createTempVariable(hoistVariableDeclaration, !!isClassWithConstructorReference);
536+
const classCheckFlags = resolver.getNodeCheckFlags(node);
537+
const isClassWithConstructorReference = classCheckFlags & NodeCheckFlags.ClassWithConstructorReference;
538+
const requiresBlockScopedVar = classCheckFlags & NodeCheckFlags.BlockScopedBindingInLoop;
539+
const temp = createTempVariable(requiresBlockScopedVar ? addBlockScopedVariable : hoistVariableDeclaration, !!isClassWithConstructorReference);
537540
if (isClassWithConstructorReference) {
538541
// record an alias as the class name is not in scope for statics.
539542
enableSubstitutionForClassAliases();
@@ -858,7 +861,6 @@ namespace ts {
858861
return undefined;
859862
}
860863

861-
862864
/**
863865
* If the name is a computed property, this function transforms it, then either returns an expression which caches the
864866
* value of the result or the expression itself if the value is either unused or safe to inline into multiple locations
@@ -872,7 +874,12 @@ namespace ts {
872874
const alreadyTransformed = isAssignmentExpression(innerExpression) && isGeneratedIdentifier(innerExpression.left);
873875
if (!alreadyTransformed && !inlinable && shouldHoist) {
874876
const generatedName = getGeneratedNameForNode(name);
875-
hoistVariableDeclaration(generatedName);
877+
if (resolver.getNodeCheckFlags(name) & NodeCheckFlags.BlockScopedBindingInLoop) {
878+
addBlockScopedVariable(generatedName);
879+
}
880+
else {
881+
hoistVariableDeclaration(generatedName);
882+
}
876883
return createAssignment(generatedName, expression);
877884
}
878885
return (inlinable || isIdentifier(innerExpression)) ? undefined : expression;
@@ -892,7 +899,12 @@ namespace ts {
892899
const text = getTextOfPropertyName(name) as string;
893900
const weakMapName = createOptimisticUniqueName("_" + text.substring(1));
894901
weakMapName.autoGenerateFlags |= GeneratedIdentifierFlags.ReservedInNestedScopes;
895-
hoistVariableDeclaration(weakMapName);
902+
if (resolver.getNodeCheckFlags(name) & NodeCheckFlags.BlockScopedBindingInLoop) {
903+
addBlockScopedVariable(weakMapName);
904+
}
905+
else {
906+
hoistVariableDeclaration(weakMapName);
907+
}
896908
(currentPrivateIdentifierEnvironment || (currentPrivateIdentifierEnvironment = createUnderscoreEscapedMap()))
897909
.set(name.escapedText, { placement: PrivateIdentifierPlacement.InstanceField, weakMapName });
898910
(pendingExpressions || (pendingExpressions = [])).push(

‎src/compiler/transformers/es2017.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ namespace ts {
223223
? visitVariableDeclarationListWithCollidingNames(node.initializer, /*hasReceiver*/ true)!
224224
: visitNode(node.initializer, visitor, isForInitializer),
225225
visitNode(node.expression, visitor, isExpression),
226-
visitNode(node.statement, asyncBodyVisitor, isStatement, liftToBlock)
226+
visitIterationBody(node.statement, asyncBodyVisitor, context)
227227
);
228228
}
229229

@@ -235,7 +235,7 @@ namespace ts {
235235
? visitVariableDeclarationListWithCollidingNames(node.initializer, /*hasReceiver*/ true)!
236236
: visitNode(node.initializer, visitor, isForInitializer),
237237
visitNode(node.expression, visitor, isExpression),
238-
visitNode(node.statement, asyncBodyVisitor, isStatement, liftToBlock)
238+
visitIterationBody(node.statement, asyncBodyVisitor, context)
239239
);
240240
}
241241

@@ -248,7 +248,7 @@ namespace ts {
248248
: visitNode(node.initializer, visitor, isForInitializer),
249249
visitNode(node.condition, visitor, isExpression),
250250
visitNode(node.incrementor, visitor, isExpression),
251-
visitNode(node.statement, asyncBodyVisitor, isStatement, liftToBlock)
251+
visitIterationBody(node.statement, asyncBodyVisitor, context)
252252
);
253253
}
254254

‎src/compiler/transformers/es2018.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ namespace ts {
427427
visitNode(node.initializer, visitorNoDestructuringValue, isForInitializer),
428428
visitNode(node.condition, visitor, isExpression),
429429
visitNode(node.incrementor, visitor, isExpression),
430-
visitNode(node.statement, visitor, isStatement)
430+
visitIterationBody(node.statement, visitor, context)
431431
);
432432
}
433433

@@ -500,7 +500,7 @@ namespace ts {
500500
let bodyLocation: TextRange | undefined;
501501
let statementsLocation: TextRange | undefined;
502502
const statements: Statement[] = [visitNode(binding, visitor, isStatement)];
503-
const statement = visitNode(node.statement, visitor, isStatement);
503+
const statement = visitIterationBody(node.statement, visitor, context);
504504
if (isBlock(statement)) {
505505
addRange(statements, statement.statements);
506506
bodyLocation = statement;

‎src/compiler/transformers/generators.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1472,7 +1472,7 @@ namespace ts {
14721472
: undefined,
14731473
visitNode(node.condition, visitor, isExpression),
14741474
visitNode(node.incrementor, visitor, isExpression),
1475-
visitNode(node.statement, visitor, isStatement, liftToBlock)
1475+
visitIterationBody(node.statement, visitor, context)
14761476
);
14771477
}
14781478
else {

‎src/compiler/transformers/module/system.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1258,7 +1258,7 @@ namespace ts {
12581258
node.initializer && visitForInitializer(node.initializer),
12591259
visitNode(node.condition, destructuringAndImportCallVisitor, isExpression),
12601260
visitNode(node.incrementor, destructuringAndImportCallVisitor, isExpression),
1261-
visitNode(node.statement, nestedElementVisitor, isStatement)
1261+
visitIterationBody(node.statement, nestedElementVisitor, context)
12621262
);
12631263

12641264
enclosingBlockScopedContainer = savedEnclosingBlockScopedContainer;
@@ -1278,7 +1278,7 @@ namespace ts {
12781278
node,
12791279
visitForInitializer(node.initializer),
12801280
visitNode(node.expression, destructuringAndImportCallVisitor, isExpression),
1281-
visitNode(node.statement, nestedElementVisitor, isStatement, liftToBlock)
1281+
visitIterationBody(node.statement, nestedElementVisitor, context)
12821282
);
12831283

12841284
enclosingBlockScopedContainer = savedEnclosingBlockScopedContainer;
@@ -1299,7 +1299,7 @@ namespace ts {
12991299
node.awaitModifier,
13001300
visitForInitializer(node.initializer),
13011301
visitNode(node.expression, destructuringAndImportCallVisitor, isExpression),
1302-
visitNode(node.statement, nestedElementVisitor, isStatement, liftToBlock)
1302+
visitIterationBody(node.statement, nestedElementVisitor, context)
13031303
);
13041304

13051305
enclosingBlockScopedContainer = savedEnclosingBlockScopedContainer;
@@ -1347,7 +1347,7 @@ namespace ts {
13471347
function visitDoStatement(node: DoStatement): VisitResult<Statement> {
13481348
return updateDo(
13491349
node,
1350-
visitNode(node.statement, nestedElementVisitor, isStatement, liftToBlock),
1350+
visitIterationBody(node.statement, nestedElementVisitor, context),
13511351
visitNode(node.expression, destructuringAndImportCallVisitor, isExpression)
13521352
);
13531353
}
@@ -1361,7 +1361,7 @@ namespace ts {
13611361
return updateWhile(
13621362
node,
13631363
visitNode(node.expression, destructuringAndImportCallVisitor, isExpression),
1364-
visitNode(node.statement, nestedElementVisitor, isStatement, liftToBlock)
1364+
visitIterationBody(node.statement, nestedElementVisitor, context)
13651365
);
13661366
}
13671367

‎src/compiler/types.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5929,6 +5929,12 @@ namespace ts {
59295929
/** Hoists a variable declaration to the containing scope. */
59305930
hoistVariableDeclaration(node: Identifier): void;
59315931

5932+
/*@internal*/ startBlockScope(): void;
5933+
5934+
/*@internal*/ endBlockScope(): Statement[] | undefined;
5935+
5936+
/*@internal*/ addBlockScopedVariable(node: Identifier): void;
5937+
59325938
/** Records a request for a non-scoped emit helper in the current context. */
59335939
requestEmitHelper(helper: EmitHelper): void;
59345940

‎src/compiler/visitor.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,25 @@ namespace ts {
549549
return <Statement>singleOrUndefined(nodes) || createBlock(<NodeArray<Statement>>nodes);
550550
}
551551

552+
export function visitIterationBody(body: Statement, visitor: Visitor, context: TransformationContext): Statement {
553+
context.startBlockScope();
554+
const updated = visitNode(body, visitor, isStatement, liftToBlock);
555+
const declarations = context.endBlockScope();
556+
if (some(declarations)) {
557+
if (isBlock(updated)) {
558+
declarations.push(...updated.statements);
559+
}
560+
else {
561+
declarations.push(updated);
562+
}
563+
return setTextRange(
564+
setOriginalNode(createBlock(declarations), body),
565+
body
566+
);
567+
}
568+
return updated;
569+
}
570+
552571
/**
553572
* Aggregates the TransformFlags for a Node and its subtree.
554573
*/

0 commit comments

Comments
 (0)