Skip to content
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.

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

Downlevel emit for let\const #2161

Merged
merged 24 commits into from
Feb 28, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a0bcd7e
initial revision of downlevel compilation for let/const bindings
vladima Feb 14, 2015
b28d72a
Merge branch 'master' into letConstES5Minus
vladima Feb 14, 2015
ba52d60
try only names generated in current scope with testing if name is unique
vladima Feb 14, 2015
7f5fb8b
drop locals in block-scope container nodes during binding
vladima Feb 15, 2015
5f2588f
show error if block scoped variable declared in the loop is captured …
vladima Feb 16, 2015
4aff9c3
explicitly initialize let binding in generated code to default value
vladima Feb 16, 2015
40bcad9
accepted baselines
vladima Feb 17, 2015
83b0ddc
merge with master
vladima Feb 18, 2015
393b95e
accepted baselines
vladima Feb 18, 2015
e6cfc10
added missing files
vladima Feb 18, 2015
b4c82c9
added tests, accepted baselines
vladima Feb 18, 2015
def6812
merge with master
vladima Feb 25, 2015
8891128
moved name generation logic to utilities
vladima Feb 25, 2015
33dfe50
do not emit default initializer for let\const in for-in\for-of statem…
vladima Feb 26, 2015
32aef1a
do not report error on non-initialized const bindings in for-in\for-o…
vladima Feb 26, 2015
b183f8d
added 'nodeIsSynthesized' function, use createSynthesizedNode in emit…
vladima Feb 26, 2015
4ff22a0
added SyntaxKind.ModuleDeclaration to list of block scope containers
vladima Feb 26, 2015
16378e3
do not treat property names in binding elements as block scoped varia…
vladima Feb 27, 2015
904d116
added tests
vladima Feb 27, 2015
4bf0bb6
added comments
vladima Feb 27, 2015
7be2e50
merge with master
vladima Feb 27, 2015
626b6d4
merge with master
vladima Feb 27, 2015
09d5582
merge with master
vladima Feb 27, 2015
3b3a94c
addressed PR feedback
vladima Feb 28, 2015
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
Prev Previous commit
Next Next commit
added missing files
  • Loading branch information
vladima committed Feb 18, 2015
commit e6cfc10acc90ccaf5072f0e0fd620ba2652190f1
11 changes: 5 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4939,15 +4939,14 @@ module ts {
current = current.parent;
}

if (!inFunction) {
return;
}

var current: Node = container;
Copy link
Contributor

Choose a reason for hiding this comment

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

Say what we are interested in at this point.

while (current && !isNameScopeBoundary(current)) {
if (isIterationStatement(current, /*lookInLabeledStatements*/ false)) {
getNodeLinks(current).flags |= NodeCheckFlags.BlockScopedBindingCapturedInLoop;
grammarErrorOnFirstToken(current, Diagnostics.Code_in_the_loop_captures_block_scoped_variable_0_in_closure_This_is_natively_supported_in_ECMAScript_6_or_higher, declarationNameToString(node));
if (inFunction) {
getNodeLinks(current).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
grammarErrorOnFirstToken(current, Diagnostics.Code_in_the_loop_captures_block_scoped_variable_0_in_closure_This_is_natively_supported_in_ECMAScript_6_or_higher, declarationNameToString(node));
}
getNodeLinks(<VariableDeclaration>symbol.valueDeclaration).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
break;
}
current = current.parent;
Expand Down
43 changes: 31 additions & 12 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3282,7 +3282,7 @@ module ts {
emitDestructuringAssignment(e, createElementAccess(value, createNumericLiteral(i)));
}
else {
if (i === elements.length - 1 && (<SpreadElementExpression>e).expression.kind === SyntaxKind.Identifier) {
if (i === elements.length - 1) {
value = ensureIdentifier(value);
emitAssignment(<Identifier>(<SpreadElementExpression>e).expression, value);
write(".slice(" + i + ")");
Expand Down Expand Up @@ -3384,12 +3384,23 @@ module ts {
}
}
else {
var initializeToDefault = renameNonTopLevelLetAndConst(<Identifier>node.name);
var isLet = renameNonTopLevelLetAndConst(<Identifier>node.name);
emitModuleMemberName(node);

var initializer =
node.initializer ||
(initializeToDefault && createVoidZero());
var initializer = node.initializer;
if (!initializer) {
// 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
var initializer =
languageVersion < ScriptTarget.ES6 &&
(resolver.getNodeCheckFlags(node) & NodeCheckFlags.BlockScopedBindingInLoop) &&
(getCombinedFlagsForIdentifier(<Identifier>node.name) & NodeFlags.Let) &&
createVoidZero();
}

emitOptional(" = ", initializer);
}
Expand Down Expand Up @@ -3422,29 +3433,39 @@ module ts {
}
}

function renameNonTopLevelLetAndConst(node: Node): boolean {
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 renameNonTopLevelLetAndConst(node: Node): void {
// do not rename if
// - language version is ES6+
// - node is synthesized (does not have a parent)
// - node is not identifier (can happen when tree is malformed)
// - node is definitely not name of variable declaration.
// it still can be part of parameter declaration, this check will be done next
if (languageVersion >= ScriptTarget.ES6 ||
!node.parent ||
node.kind !== SyntaxKind.Identifier ||
(node.parent.kind !== SyntaxKind.VariableDeclaration && node.parent.kind !== SyntaxKind.BindingElement)) {
return false;
return;
}

var combinedFlags = getCombinedNodeFlags(node.parent);
var combinedFlags = getCombinedFlagsForIdentifier(<Identifier>node);
if (((combinedFlags & NodeFlags.BlockScoped) === 0) || combinedFlags & NodeFlags.Export) {
// do not rename exported or non-block scoped variables
return false;
return;
}

// here it is known that node is a block scoped variable
var list = getAncestor(node, SyntaxKind.VariableDeclarationList);
if (list.parent.kind === SyntaxKind.VariableStatement && list.parent.parent.kind === SyntaxKind.SourceFile) {
// do not rename variables that are defined on source file level
return false;
return;
}

var generatedName = makeUniqueName(getEnclosingBlockScopeContainer(node), (<Identifier>node).text);
Expand All @@ -3453,8 +3474,6 @@ module ts {
generatedBlockScopeNames = [];
}
generatedBlockScopeNames[symbolId] = generatedName;

return (combinedFlags & NodeFlags.Let) !== 0;
}

function emitVariableStatement(node: VariableStatement) {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1262,7 +1262,7 @@ module ts {

// Values for enum members have been computed, and any errors have been reported for them.
EnumValuesComputed = 0x00000080,
BlockScopedBindingCapturedInLoop = 0x00000100,
BlockScopedBindingInLoop = 0x00000100,
Copy link
Contributor

Choose a reason for hiding this comment

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

loop body, not loop initializer, right?

Copy link

Choose a reason for hiding this comment

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

According to ES6 spec, let in the loop initializer is also scoped per iteration (so subject to the same downlevel codegen issues). Beware that browsers currently get it wrong (tested in IE11, FF33).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I learned that while in the middle of reviewing this PR. I still do not quite understand the difference between the loop scope and the body scope, given that they are both per iteration. I suppose the difference is that you can access the loop scoped bindings in the initializer, guard, and incrementor of the loop, but they will be shadowed as soon as the body starts for that iteration. Though I still don't see how there could be a difference for for...in and for...of.

Copy link

Choose a reason for hiding this comment

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

To be honest I don't think that there is any useful difference between the two. But there is a technical difference, as the loop body is a new scope nested inside the loop scope itself. It means that you can redeclare let i both in the loop initializer and the loop body. The initializer, loop increment and loop condition share one scope, the loop body is a nested scope.

Allen makes it very clear in his answer to this question: https://esdiscuss.org/topic/in-es6-do-for-loops-with-a-let-const-initializer-create-a-separate-scope

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, thanks for explaining that!

}

export interface NodeLinks {
Expand Down