Skip to content

Commit

Permalink
Bloomberg computed property name fix (#43197)
Browse files Browse the repository at this point in the history
* Fix property name bindings for class expr in loops

* Fix block-scope capturing with prop initializers

Co-authored-by: Joey Watts <joey.watts.96@gmail.com>
  • Loading branch information
sandersn and joeywatts authored Mar 11, 2021
1 parent 998ecd9 commit b2d1f53
Show file tree
Hide file tree
Showing 31 changed files with 587 additions and 41 deletions.
55 changes: 38 additions & 17 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23775,14 +23775,20 @@ namespace ts {
return assignmentKind ? getBaseTypeOfLiteralType(flowType) : flowType;
}

function isInsideFunction(node: Node, threshold: Node): boolean {
return !!findAncestor(node, n => n === threshold ? "quit" : isFunctionLike(n));
function isInsideFunctionOrInstancePropertyInitializer(node: Node, threshold: Node): boolean {
return !!findAncestor(node, n => n === threshold ? "quit" : isFunctionLike(n) || (
n.parent && isPropertyDeclaration(n.parent) && !hasStaticModifier(n.parent) && n.parent.initializer === n
));
}

function getPartOfForStatementContainingNode(node: Node, container: ForStatement) {
return findAncestor(node, n => n === container ? "quit" : n === container.initializer || n === container.condition || n === container.incrementor || n === container.statement);
}

function getEnclosingIterationStatement(node: Node): Node | undefined {
return findAncestor(node, n => (!n || nodeStartsNewLexicalEnvironment(n)) ? "quit" : isIterationStatement(n, /*lookInLabeledStatements*/ false));
}

function checkNestedBlockScopedBinding(node: Identifier, symbol: Symbol): void {
if (languageVersion >= ScriptTarget.ES2015 ||
(symbol.flags & (SymbolFlags.BlockScopedVariable | SymbolFlags.Class)) === 0 ||
Expand All @@ -23798,20 +23804,11 @@ namespace ts {
// if there is an iteration statement in between declaration and boundary (is binding/class declared inside iteration statement)

const container = getEnclosingBlockScopeContainer(symbol.valueDeclaration);
const usedInFunction = isInsideFunction(node.parent, container);
let current = container;

let containedInIterationStatement = false;
while (current && !nodeStartsNewLexicalEnvironment(current)) {
if (isIterationStatement(current, /*lookInLabeledStatements*/ false)) {
containedInIterationStatement = true;
break;
}
current = current.parent;
}
const isCaptured = isInsideFunctionOrInstancePropertyInitializer(node, container);

if (containedInIterationStatement) {
if (usedInFunction) {
const enclosingIterationStatement = getEnclosingIterationStatement(container);
if (enclosingIterationStatement) {
if (isCaptured) {
// mark iteration statement as containing block-scoped binding captured in some function
let capturesBlockScopeBindingInLoopBody = true;
if (isForStatement(container)) {
Expand All @@ -23832,7 +23829,7 @@ namespace ts {
}
}
if (capturesBlockScopeBindingInLoopBody) {
getNodeLinks(current).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
getNodeLinks(enclosingIterationStatement).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
}
}

Expand All @@ -23849,7 +23846,7 @@ namespace ts {
getNodeLinks(symbol.valueDeclaration).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
}

if (usedInFunction) {
if (isCaptured) {
getNodeLinks(symbol.valueDeclaration).flags |= NodeCheckFlags.CapturedBlockScopedBinding;
}
}
Expand Down Expand Up @@ -25529,6 +25526,20 @@ namespace ts {
const links = getNodeLinks(node.expression);
if (!links.resolvedType) {
links.resolvedType = checkExpression(node.expression);
// The computed property name of a non-static class field within a loop must be stored in a block-scoped binding.
// (It needs to be bound at class evaluation time.)
if (isPropertyDeclaration(node.parent) && !hasStaticModifier(node.parent) && isClassExpression(node.parent.parent)) {
const container = getEnclosingBlockScopeContainer(node.parent.parent);
const enclosingIterationStatement = getEnclosingIterationStatement(container);
if (enclosingIterationStatement) {
// The computed field name will use a block scoped binding which can be unique for each iteration of the loop.
getNodeLinks(enclosingIterationStatement).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
// The generated variable which stores the computed field name must be block-scoped.
getNodeLinks(node).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
// The generated variable which stores the class must be block-scoped.
getNodeLinks(node.parent.parent).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
}
}
// This will allow types number, string, symbol or any. It will also allow enums, the unknown
// type, and any union of these types (like string | number).
if (links.resolvedType.flags & TypeFlags.Nullable ||
Expand Down Expand Up @@ -32617,6 +32628,16 @@ namespace ts {
for (let lexicalScope = getEnclosingBlockScopeContainer(node); !!lexicalScope; lexicalScope = getEnclosingBlockScopeContainer(lexicalScope)) {
getNodeLinks(lexicalScope).flags |= NodeCheckFlags.ContainsClassWithPrivateIdentifiers;
}

// If this is a private field in a class expression inside the body of a loop,
// then we must use a block-scoped binding to store the WeakMap.
if (isClassExpression(node.parent)) {
const enclosingIterationStatement = getEnclosingIterationStatement(node.parent);
if (enclosingIterationStatement) {
getNodeLinks(node.name).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
getNodeLinks(enclosingIterationStatement).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
}
}
}
}

Expand Down
49 changes: 49 additions & 0 deletions src/compiler/transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ namespace ts {
let lexicalEnvironmentFlagsStack: LexicalEnvironmentFlags[] = [];
let lexicalEnvironmentStackOffset = 0;
let lexicalEnvironmentSuspended = false;
let blockScopedVariableDeclarationsStack: Identifier[][] = [];
let blockScopeStackOffset = 0;
let blockScopedVariableDeclarations: Identifier[];
let emitHelpers: EmitHelper[] | undefined;
let onSubstituteNode: TransformationContext["onSubstituteNode"] = noEmitSubstitution;
let onEmitNode: TransformationContext["onEmitNode"] = noEmitNotification;
Expand All @@ -182,6 +185,9 @@ namespace ts {
hoistVariableDeclaration,
hoistFunctionDeclaration,
addInitializationStatement,
startBlockScope,
endBlockScope,
addBlockScopedVariable,
requestEmitHelper,
readEmitHelpers,
enableSubstitution,
Expand Down Expand Up @@ -473,6 +479,46 @@ namespace ts {
return lexicalEnvironmentFlags;
}

/**
* Starts a block scope. Any existing block hoisted variables are pushed onto the stack and the related storage variables are reset.
*/
function startBlockScope() {
Debug.assert(state > TransformationState.Uninitialized, "Cannot start a block scope during initialization.");
Debug.assert(state < TransformationState.Completed, "Cannot start a block scope after transformation has completed.");
blockScopedVariableDeclarationsStack[blockScopeStackOffset] = blockScopedVariableDeclarations;
blockScopeStackOffset++;
blockScopedVariableDeclarations = undefined!;
}

/**
* Ends a block scope. The previous set of block hoisted variables are restored. Any hoisted declarations are returned.
*/
function endBlockScope() {
Debug.assert(state > TransformationState.Uninitialized, "Cannot end a block scope during initialization.");
Debug.assert(state < TransformationState.Completed, "Cannot end a block scope after transformation has completed.");
const statements: Statement[] | undefined = some(blockScopedVariableDeclarations) ?
[
factory.createVariableStatement(
/*modifiers*/ undefined,
factory.createVariableDeclarationList(
blockScopedVariableDeclarations.map(identifier => factory.createVariableDeclaration(identifier)),
NodeFlags.Let
)
)
] : undefined;
blockScopeStackOffset--;
blockScopedVariableDeclarations = blockScopedVariableDeclarationsStack[blockScopeStackOffset];
if (blockScopeStackOffset === 0) {
blockScopedVariableDeclarationsStack = [];
}
return statements;
}

function addBlockScopedVariable(name: Identifier): void {
Debug.assert(blockScopeStackOffset > 0, "Cannot add a block scoped variable outside of an iteration body.");
(blockScopedVariableDeclarations || (blockScopedVariableDeclarations = [])).push(name);
}

function requestEmitHelper(helper: EmitHelper): void {
Debug.assert(state > TransformationState.Uninitialized, "Cannot modify the transformation context during initialization.");
Debug.assert(state < TransformationState.Completed, "Cannot modify the transformation context after transformation has completed.");
Expand Down Expand Up @@ -539,5 +585,8 @@ namespace ts {
startLexicalEnvironment: noop,
suspendLexicalEnvironment: noop,
addDiagnostic: noop,
startBlockScope: noop,
endBlockScope: returnUndefined,
addBlockScopedVariable: noop
};
}
26 changes: 19 additions & 7 deletions src/compiler/transformers/classFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ namespace ts {
factory,
hoistVariableDeclaration,
endLexicalEnvironment,
resumeLexicalEnvironment
resumeLexicalEnvironment,
addBlockScopedVariable
} = context;
const resolver = context.getEmitResolver();
const compilerOptions = context.getCompilerOptions();
Expand Down Expand Up @@ -310,7 +311,7 @@ namespace ts {
visitNode(node.initializer, visitor, isForInitializer),
visitNode(node.condition, visitor, isExpression),
visitPostfixUnaryExpression(node.incrementor, /*valueIsDiscarded*/ true),
visitNode(node.statement, visitor, isStatement)
visitIterationBody(node.statement, visitor, context)
);
}
return visitEachChild(node, visitor, context);
Expand Down Expand Up @@ -540,8 +541,10 @@ namespace ts {
}
else {
const expressions: Expression[] = [];
const isClassWithConstructorReference = resolver.getNodeCheckFlags(node) & NodeCheckFlags.ClassWithConstructorReference;
const temp = factory.createTempVariable(hoistVariableDeclaration, !!isClassWithConstructorReference);
const classCheckFlags = resolver.getNodeCheckFlags(node);
const isClassWithConstructorReference = classCheckFlags & NodeCheckFlags.ClassWithConstructorReference;
const requiresBlockScopedVar = classCheckFlags & NodeCheckFlags.BlockScopedBindingInLoop;
const temp = factory.createTempVariable(requiresBlockScopedVar ? addBlockScopedVariable : hoistVariableDeclaration, !!isClassWithConstructorReference);
if (isClassWithConstructorReference) {
// record an alias as the class name is not in scope for statics.
enableSubstitutionForClassAliases();
Expand Down Expand Up @@ -869,7 +872,6 @@ namespace ts {
return undefined;
}


/**
* If the name is a computed property, this function transforms it, then either returns an expression which caches the
* value of the result or the expression itself if the value is either unused or safe to inline into multiple locations
Expand All @@ -883,7 +885,12 @@ namespace ts {
const alreadyTransformed = isAssignmentExpression(innerExpression) && isGeneratedIdentifier(innerExpression.left);
if (!alreadyTransformed && !inlinable && shouldHoist) {
const generatedName = factory.getGeneratedNameForNode(name);
hoistVariableDeclaration(generatedName);
if (resolver.getNodeCheckFlags(name) & NodeCheckFlags.BlockScopedBindingInLoop) {
addBlockScopedVariable(generatedName);
}
else {
hoistVariableDeclaration(generatedName);
}
return factory.createAssignment(generatedName, expression);
}
return (inlinable || isIdentifier(innerExpression)) ? undefined : expression;
Expand All @@ -910,7 +917,12 @@ namespace ts {
function addPrivateIdentifierToEnvironment(name: PrivateIdentifier) {
const text = getTextOfPropertyName(name) as string;
const weakMapName = factory.createUniqueName("_" + text.substring(1), GeneratedIdentifierFlags.Optimistic | GeneratedIdentifierFlags.ReservedInNestedScopes);
hoistVariableDeclaration(weakMapName);
if (resolver.getNodeCheckFlags(name) & NodeCheckFlags.BlockScopedBindingInLoop) {
addBlockScopedVariable(weakMapName);
}
else {
hoistVariableDeclaration(weakMapName);
}
getPrivateIdentifierEnvironment().set(name.escapedText, { placement: PrivateIdentifierPlacement.InstanceField, weakMapName });
getPendingExpressions().push(
factory.createAssignment(
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/transformers/es2017.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ namespace ts {
? visitVariableDeclarationListWithCollidingNames(node.initializer, /*hasReceiver*/ true)!
: visitNode(node.initializer, visitor, isForInitializer),
visitNode(node.expression, visitor, isExpression),
visitNode(node.statement, asyncBodyVisitor, isStatement, factory.liftToBlock)
visitIterationBody(node.statement, asyncBodyVisitor, context)
);
}

Expand All @@ -237,7 +237,7 @@ namespace ts {
? visitVariableDeclarationListWithCollidingNames(node.initializer, /*hasReceiver*/ true)!
: visitNode(node.initializer, visitor, isForInitializer),
visitNode(node.expression, visitor, isExpression),
visitNode(node.statement, asyncBodyVisitor, isStatement, factory.liftToBlock)
visitIterationBody(node.statement, asyncBodyVisitor, context)
);
}

Expand All @@ -250,7 +250,7 @@ namespace ts {
: visitNode(node.initializer, visitor, isForInitializer),
visitNode(node.condition, visitor, isExpression),
visitNode(node.incrementor, visitor, isExpression),
visitNode(node.statement, asyncBodyVisitor, isStatement, factory.liftToBlock)
visitIterationBody(node.statement, asyncBodyVisitor, context)
);
}

Expand Down
4 changes: 2 additions & 2 deletions src/compiler/transformers/es2018.ts
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ namespace ts {
visitNode(node.initializer, visitorWithUnusedExpressionResult, isForInitializer),
visitNode(node.condition, visitor, isExpression),
visitNode(node.incrementor, visitorWithUnusedExpressionResult, isExpression),
visitNode(node.statement, visitor, isStatement)
visitIterationBody(node.statement, visitor, context)
);
}

Expand Down Expand Up @@ -648,7 +648,7 @@ namespace ts {
let bodyLocation: TextRange | undefined;
let statementsLocation: TextRange | undefined;
const statements: Statement[] = [visitNode(binding, visitor, isStatement)];
const statement = visitNode(node.statement, visitor, isStatement);
const statement = visitIterationBody(node.statement, visitor, context);
if (isBlock(statement)) {
addRange(statements, statement.statements);
bodyLocation = statement;
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/transformers/generators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1502,7 +1502,7 @@ namespace ts {
: undefined,
visitNode(node.condition, visitor, isExpression),
visitNode(node.incrementor, visitor, isExpression),
visitNode(node.statement, visitor, isStatement, factory.liftToBlock)
visitIterationBody(node.statement, visitor, context)
);
}
else {
Expand Down
10 changes: 5 additions & 5 deletions src/compiler/transformers/module/system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,7 @@ namespace ts {
node.initializer && visitForInitializer(node.initializer),
visitNode(node.condition, destructuringAndImportCallVisitor, isExpression),
visitNode(node.incrementor, destructuringAndImportCallVisitor, isExpression),
visitNode(node.statement, nestedElementVisitor, isStatement)
visitIterationBody(node.statement, nestedElementVisitor, context)
);

enclosingBlockScopedContainer = savedEnclosingBlockScopedContainer;
Expand All @@ -1251,7 +1251,7 @@ namespace ts {
node,
visitForInitializer(node.initializer),
visitNode(node.expression, destructuringAndImportCallVisitor, isExpression),
visitNode(node.statement, nestedElementVisitor, isStatement, factory.liftToBlock)
visitIterationBody(node.statement, nestedElementVisitor, context)
);

enclosingBlockScopedContainer = savedEnclosingBlockScopedContainer;
Expand All @@ -1272,7 +1272,7 @@ namespace ts {
node.awaitModifier,
visitForInitializer(node.initializer),
visitNode(node.expression, destructuringAndImportCallVisitor, isExpression),
visitNode(node.statement, nestedElementVisitor, isStatement, factory.liftToBlock)
visitIterationBody(node.statement, nestedElementVisitor, context)
);

enclosingBlockScopedContainer = savedEnclosingBlockScopedContainer;
Expand Down Expand Up @@ -1320,7 +1320,7 @@ namespace ts {
function visitDoStatement(node: DoStatement): VisitResult<Statement> {
return factory.updateDoStatement(
node,
visitNode(node.statement, nestedElementVisitor, isStatement, factory.liftToBlock),
visitIterationBody(node.statement, nestedElementVisitor, context),
visitNode(node.expression, destructuringAndImportCallVisitor, isExpression)
);
}
Expand All @@ -1334,7 +1334,7 @@ namespace ts {
return factory.updateWhileStatement(
node,
visitNode(node.expression, destructuringAndImportCallVisitor, isExpression),
visitNode(node.statement, nestedElementVisitor, isStatement, factory.liftToBlock)
visitIterationBody(node.statement, nestedElementVisitor, context)
);
}

Expand Down
Loading

0 comments on commit b2d1f53

Please sign in to comment.