Skip to content

handle block scoped binding in nested blocks #6553

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
Jan 27, 2016
Merged

handle block scoped binding in nested blocks #6553

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
a2c655d
handle block scoped binding in nested blocks
vladima Jan 20, 2016
e67ff39
merge with origin/master
vladima Jan 27, 2016
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
120 changes: 65 additions & 55 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7156,7 +7156,7 @@ namespace ts {

checkCollisionWithCapturedSuperVariable(node, node);
checkCollisionWithCapturedThisVariable(node, node);
checkBlockScopedBindingCapturedInLoop(node, symbol);
checkNestedBlockScopedBinding(node, symbol);

return getNarrowedTypeOfSymbol(getExportSymbolOfValueSymbolIfExported(symbol), node);
}
Expand All @@ -7173,7 +7173,7 @@ namespace ts {
return false;
}

function checkBlockScopedBindingCapturedInLoop(node: Identifier, symbol: Symbol): void {
function checkNestedBlockScopedBinding(node: Identifier, symbol: Symbol): void {
if (languageVersion >= ScriptTarget.ES6 ||
(symbol.flags & (SymbolFlags.BlockScopedVariable | SymbolFlags.Class)) === 0 ||
symbol.valueDeclaration.parent.kind === SyntaxKind.CatchClause) {
Expand All @@ -7185,40 +7185,31 @@ namespace ts {
// 2. walk from the declaration up to the boundary of lexical environment and check
// if there is an iteration statement in between declaration and boundary (is binding/class declared inside iteration statement)

let container: Node;
if (symbol.flags & SymbolFlags.Class) {
// get parent of class declaration
container = getClassLikeDeclarationOfSymbol(symbol).parent;
}
else {
// nesting structure:
// (variable declaration or binding element) -> variable declaration list -> container
container = symbol.valueDeclaration;
while (container.kind !== SyntaxKind.VariableDeclarationList) {
container = container.parent;
}
// get the parent of variable declaration list
container = container.parent;
if (container.kind === SyntaxKind.VariableStatement) {
// if parent is variable statement - get its parent
container = container.parent;
}
}

const inFunction = isInsideFunction(node.parent, container);

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)) {
if (inFunction) {
getNodeLinks(current).flags |= NodeCheckFlags.LoopWithBlockScopedBindingCapturedInFunction;
}
// mark value declaration so during emit they can have a special handling
getNodeLinks(<VariableDeclaration>symbol.valueDeclaration).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
containedInIterationStatement = true;
break;
}
current = current.parent;
}

if (containedInIterationStatement) {
if (usedInFunction) {
// mark iteration statement as containing block-scoped binding captured in some function
getNodeLinks(current).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
}
// set 'declared inside loop' bit on the block-scoped binding
getNodeLinks(symbol.valueDeclaration).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
}

if (usedInFunction) {
getNodeLinks(symbol.valueDeclaration).flags |= NodeCheckFlags.CapturedBlockScopedBinding;
}
}

function captureLexicalThis(node: Node, container: Node): void {
Expand Down Expand Up @@ -15623,42 +15614,61 @@ namespace ts {
return symbol && symbol.flags & SymbolFlags.Alias ? getDeclarationOfAliasSymbol(symbol) : undefined;
}

function isStatementWithLocals(node: Node) {
switch (node.kind) {
case SyntaxKind.Block:
case SyntaxKind.CaseBlock:
case SyntaxKind.ForStatement:
case SyntaxKind.ForInStatement:
case SyntaxKind.ForOfStatement:
return true;
}
return false;
}

function isNestedRedeclarationSymbol(symbol: Symbol): boolean {
function isSymbolOfDeclarationWithCollidingName(symbol: Symbol): boolean {
if (symbol.flags & SymbolFlags.BlockScoped) {
const links = getSymbolLinks(symbol);
if (links.isNestedRedeclaration === undefined) {
if (links.isDeclaratonWithCollidingName === undefined) {
const container = getEnclosingBlockScopeContainer(symbol.valueDeclaration);
links.isNestedRedeclaration = isStatementWithLocals(container) &&
!!resolveName(container.parent, symbol.name, SymbolFlags.Value, /*nameNotFoundMessage*/ undefined, /*nameArg*/ undefined);
if (isStatementWithLocals(container)) {
const nodeLinks = getNodeLinks(symbol.valueDeclaration);
if (!!resolveName(container.parent, symbol.name, SymbolFlags.Value, /*nameNotFoundMessage*/ undefined, /*nameArg*/ undefined)) {
// redeclaration - always should be renamed
links.isDeclaratonWithCollidingName = true;
}
else if (nodeLinks.flags & NodeCheckFlags.CapturedBlockScopedBinding) {
// binding is captured in the function
// should be renamed if:
// - binding is not top level - top level bindings never collide with anything
// AND
// - binding is not declared in loop, should be renamed to avoid name reuse across siblings
// let a, b
// { let x = 1; a = () => x; }
// { let x = 100; b = () => x; }
// console.log(a()); // should print '1'
// console.log(b()); // should print '100'
// OR
// - binding is declared inside loop but not in inside initializer of iteration statement or directly inside loop body
// * variables from initializer are passed to rewritted loop body as parameters so they are not captured directly
// * variables that are declared immediately in loop body will become top level variable after loop is rewritten and thus
// they will not collide with anything
const isDeclaredInLoop = nodeLinks.flags & NodeCheckFlags.BlockScopedBindingInLoop;
const inLoopInitializer = isIterationStatement(container, /*lookInLabeledStatements*/ false);
const inLoopBodyBlock = container.kind === SyntaxKind.Block && isIterationStatement(container.parent, /*lookInLabeledStatements*/ false);

links.isDeclaratonWithCollidingName = !isBlockScopedContainerTopLevel(container) && (!isDeclaredInLoop || (!inLoopInitializer && !inLoopBodyBlock));
}
else {
links.isDeclaratonWithCollidingName = false;
}
}
}
return links.isNestedRedeclaration;
return links.isDeclaratonWithCollidingName;
}
return false;
}

// When resolved as an expression identifier, if the given node references a nested block scoped entity with
// a name that hides an existing name, return the declaration of that entity. Otherwise, return undefined.
function getReferencedNestedRedeclaration(node: Identifier): Declaration {
// a name that either hides an existing name or might hide it when compiled downlevel,
// return the declaration of that entity. Otherwise, return undefined.
function getReferencedDeclarationWithCollidingName(node: Identifier): Declaration {
const symbol = getReferencedValueSymbol(node);
return symbol && isNestedRedeclarationSymbol(symbol) ? symbol.valueDeclaration : undefined;
return symbol && isSymbolOfDeclarationWithCollidingName(symbol) ? symbol.valueDeclaration : undefined;
}

// Return true if the given node is a declaration of a nested block scoped entity with a name that hides an
// existing name.
function isNestedRedeclaration(node: Declaration): boolean {
return isNestedRedeclarationSymbol(getSymbolOfNode(node));
// Return true if the given node is a declaration of a nested block scoped entity with a name that either hides an
// existing name or might hide a name when compiled downlevel
function isDeclarationWithCollidingName(node: Declaration): boolean {
return isSymbolOfDeclarationWithCollidingName(getSymbolOfNode(node));
}

function isValueAliasDeclaration(node: Node): boolean {
Expand Down Expand Up @@ -15859,8 +15869,8 @@ namespace ts {
return {
getReferencedExportContainer,
getReferencedImportDeclaration,
getReferencedNestedRedeclaration,
isNestedRedeclaration,
getReferencedDeclarationWithCollidingName,
isDeclarationWithCollidingName,
isValueAliasDeclaration,
hasGlobalName,
isReferencedAliasDeclaration,
Expand Down
86 changes: 56 additions & 30 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1530,7 +1530,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
}

if (languageVersion !== ScriptTarget.ES6) {
const declaration = resolver.getReferencedNestedRedeclaration(node);
const declaration = resolver.getReferencedDeclarationWithCollidingName(node);
if (declaration) {
write(getGeneratedNameForNode(declaration.name));
return;
Expand All @@ -1546,15 +1546,15 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
}
}

function isNameOfNestedRedeclaration(node: Identifier) {
function isNameOfNestedBlockScopedRedeclarationOrCapturedBinding(node: Identifier) {
if (languageVersion < ScriptTarget.ES6) {
const parent = node.parent;
switch (parent.kind) {
case SyntaxKind.BindingElement:
case SyntaxKind.ClassDeclaration:
case SyntaxKind.EnumDeclaration:
case SyntaxKind.VariableDeclaration:
return (<Declaration>parent).name === node && resolver.isNestedRedeclaration(<Declaration>parent);
return (<Declaration>parent).name === node && resolver.isDeclarationWithCollidingName(<Declaration>parent);
}
}
return false;
Expand All @@ -1576,7 +1576,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
else if (isExpressionIdentifier(node)) {
emitExpressionIdentifier(node);
}
else if (isNameOfNestedRedeclaration(node)) {
else if (isNameOfNestedBlockScopedRedeclarationOrCapturedBinding(node)) {
write(getGeneratedNameForNode(node));
}
else if (nodeIsSynthesized(node)) {
Expand Down Expand Up @@ -2891,7 +2891,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge

function shouldConvertLoopBody(node: IterationStatement): boolean {
return languageVersion < ScriptTarget.ES6 &&
(resolver.getNodeCheckFlags(node) & NodeCheckFlags.LoopWithBlockScopedBindingCapturedInFunction) !== 0;
(resolver.getNodeCheckFlags(node) & NodeCheckFlags.LoopWithCapturedBlockScopedBinding) !== 0;
}

function emitLoop(node: IterationStatement, loopEmitter: (n: IterationStatement, convertedLoop: ConvertedLoop) => void): void {
Expand Down Expand Up @@ -3045,7 +3045,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge

function collectNames(name: Identifier | BindingPattern): void {
if (name.kind === SyntaxKind.Identifier) {
const nameText = isNameOfNestedRedeclaration(<Identifier>name) ? getGeneratedNameForNode(name) : (<Identifier>name).text;
const nameText = isNameOfNestedBlockScopedRedeclarationOrCapturedBinding(<Identifier>name) ? getGeneratedNameForNode(name) : (<Identifier>name).text;
loopParameters.push(nameText);
}
else {
Expand Down Expand Up @@ -4065,22 +4065,56 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
}
else {
let initializer = node.initializer;
if (!initializer && languageVersion < ScriptTarget.ES6) {

// downlevel emit for non-initialized let bindings defined in loops
// for (...) { let x; }
// should be
// for (...) { var <some-uniqie-name> = void 0; }
// this is necessary to preserve ES6 semantic in scenarios like
// for (...) { let x; console.log(x); x = 1 } // assignment on one iteration should not affect other iterations
const isLetDefinedInLoop =
(resolver.getNodeCheckFlags(node) & NodeCheckFlags.BlockScopedBindingInLoop) &&
(getCombinedFlagsForIdentifier(<Identifier>node.name) & NodeFlags.Let);

// NOTE: default initialization should not be added to let bindings in for-in\for-of statements
if (isLetDefinedInLoop &&
node.parent.parent.kind !== SyntaxKind.ForInStatement &&
node.parent.parent.kind !== SyntaxKind.ForOfStatement) {
if (!initializer &&
languageVersion < ScriptTarget.ES6 &&
// for names - binding patterns that lack initializer there is no point to emit explicit initializer
// since downlevel codegen for destructuring will fail in the absence of initializer so all binding elements will say uninitialized
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a hard time parsing this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment is related to our emit in the presence of errors, should we emit zero initializers for cases like this one

let {a,b,c};

Currently we don't do this because main reason why it is done for identifiers is that we don't want old value to be observed. However for destructuring downlevel emit will always throw if initializer is omitted .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Agreed, we shouldn't emit initializers in the destructuring case.

node.name.kind === SyntaxKind.Identifier) {

const container = getEnclosingBlockScopeContainer(node);
const flags = resolver.getNodeCheckFlags(node);

// nested let bindings might need to be initialized explicitly to preserve ES6 semantic
// { let x = 1; }
// { let x; } // x here should be undefined. not 1
// NOTES:
// Top level bindings never collide with anything and thus don't require explicit initialization.
// As for nested let bindings there are two cases:
// - nested let bindings that were not renamed definitely should be initialized explicitly
// { let x = 1; }
// { let x; if (some-condition) { x = 1}; if (x) { /*1*/ } }
// Without explicit initialization code in /*1*/ can be executed even if some-condition is evaluated to false
// - renaming introduces fresh name that should not collide with any existing names, however renamed bindings sometimes also should be
// explicitly initialized. One particular case: non-captured binding declared inside loop body (but not in loop initializer)
// let x;
// for (;;) {
// let x;
// }
// in downlevel codegen inner 'x' will be renamed so it won't collide with outer 'x' however it will should be reset on every iteration
// as if it was declared anew.
// * Why non-captured binding - because if loop contains block scoped binding captured in some function then loop body will be rewritten
// to have a fresh scope on every iteration so everything will just work.
// * Why loop initializer is excluded - since we've introduced a fresh name it already will be undefined.
const isCapturedInFunction = flags & NodeCheckFlags.CapturedBlockScopedBinding;
const isDeclaredInLoop = flags & NodeCheckFlags.BlockScopedBindingInLoop;

const emittedAsTopLevel =
isBlockScopedContainerTopLevel(container) ||
(isCapturedInFunction && isDeclaredInLoop && container.kind === SyntaxKind.Block && isIterationStatement(container.parent, /*lookInLabeledStatements*/ false));

const emittedAsNestedLetDeclaration =
getCombinedNodeFlags(node) & NodeFlags.Let &&
!emittedAsTopLevel;

const emitExplicitInitializer =
emittedAsNestedLetDeclaration &&
container.kind !== SyntaxKind.ForInStatement &&
container.kind !== SyntaxKind.ForOfStatement &&
(
!resolver.isDeclarationWithCollidingName(node) ||
(isDeclaredInLoop && !isCapturedInFunction && !isIterationStatement(container, /*lookInLabeledStatements*/ false))
);
if (emitExplicitInitializer) {
initializer = createVoidZero();
}
}
Expand Down Expand Up @@ -4115,14 +4149,6 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
}
}

function getCombinedFlagsForIdentifier(node: Identifier): NodeFlags {
if (!node.parent || (node.parent.kind !== SyntaxKind.VariableDeclaration && node.parent.kind !== SyntaxKind.BindingElement)) {
return 0;
}

return getCombinedNodeFlags(node.parent);
}

function isES6ExportedDeclaration(node: Node) {
return !!(node.flags & NodeFlags.Export) &&
modulekind === ModuleKind.ES6 &&
Expand Down
Loading