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 23 commits
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
16 changes: 14 additions & 2 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ module ts {

if (!file.locals) {
file.locals = {};
container = blockScopeContainer = file;
container = file;
setBlockScopeContainer(file, /*cleanLocals*/ false);
bind(file);
file.symbolCount = symbolCount;
}
Expand All @@ -77,6 +78,13 @@ module ts {
return new Symbol(flags, name);
}

function setBlockScopeContainer(node: Node, cleanLocals: boolean) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't know what cleanLocals means without reading the function, and even then I don't know what the implications for this are. Can you leave a comment regarding the situations in which this is useful?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand this. Why is blockScopedContainer.locals populated? I thought it should always be empty to start.

blockScopeContainer = node;
if (cleanLocals) {
blockScopeContainer.locals = undefined;
}
}

function addDeclarationToSymbol(symbol: Symbol, node: Declaration, symbolKind: SymbolFlags) {
symbol.flags |= symbolKind;
if (!symbol.declarations) symbol.declarations = [];
Expand Down Expand Up @@ -236,7 +244,10 @@ module ts {
}

if (isBlockScopeContainer) {
blockScopeContainer = node;
// clean locals in block scope container if
// - current node does not have local variables
// - current node is not source file (source file always have locals)
setBlockScopeContainer(node, /*cleanLocals*/ (symbolKind & SymbolFlags.HasLocals) == 0 && node.kind !== SyntaxKind.SourceFile);
Copy link
Member

Choose a reason for hiding this comment

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

Remove extra space after the comment

Copy link
Contributor

Choose a reason for hiding this comment

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

"==="

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

forEachChild(node, bind);
Expand Down Expand Up @@ -343,6 +354,7 @@ module ts {

function bindCatchVariableDeclaration(node: CatchClause) {
bindChildren(node, /*symbolKind:*/ 0, /*isBlockScopeContainer:*/ true);

Copy link
Member

Choose a reason for hiding this comment

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

Remove extra line.

Copy link
Contributor

Choose a reason for hiding this comment

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

remove space

}

function bindBlockScopedVariableDeclaration(node: Declaration) {
Expand Down
118 changes: 89 additions & 29 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5075,10 +5075,59 @@ module ts {

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

return getNarrowedTypeOfSymbol(getExportSymbolOfValueSymbolIfExported(symbol), node);
}

function checkBlockScopedBindingCapturedInLoop(node: Identifier, symbol: Symbol): void {
if (languageVersion >= ScriptTarget.ES6 ||
(symbol.flags & SymbolFlags.BlockScopedVariable) === 0 ||
symbol.valueDeclaration.parent.kind === SyntaxKind.CatchClause) {
return;
}

// - check if binding is used in some function
// (stop the walk when reaching container of binding declaration)
// - if first check succeeded - check if variable is declared inside the loop

// nesting structure:
// (variable declaration or binding element) -> variable declaration list -> container
var container: Node = 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;
}

var inFunction = false;
var current = node.parent;
while (current && current !== container) {
if (isAnyFunction(current)) {
inFunction = true;
break;
}
current = current.parent;
}

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 && !nodeStartsNewLexicalEnvironment(current)) {
if (isIterationStatement(current, /*lookInLabeledStatements*/ false)) {
if (inFunction) {
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));
}
// mark value declaration so during emit they can have a special handling
getNodeLinks(<VariableDeclaration>symbol.valueDeclaration).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
break;
}
current = current.parent;
}
}

function captureLexicalThis(node: Node, container: Node): void {
var classNode = container.parent && container.parent.kind === SyntaxKind.ClassDeclaration ? container.parent : undefined;
getNodeLinks(node).flags |= NodeCheckFlags.LexicalThis;
Expand Down Expand Up @@ -10467,25 +10516,8 @@ module ts {
}

function makeUniqueName(baseName: string): string {
// First try '_name'
if (baseName.charCodeAt(0) !== CharacterCodes._) {
var baseName = "_" + baseName;
if (!isExistingName(baseName)) {
return generatedNames[baseName] = baseName;
}
}
// Find the first unique '_name_n', where n is a positive number
if (baseName.charCodeAt(baseName.length - 1) !== CharacterCodes._) {
baseName += "_";
}
var i = 1;
while (true) {
name = baseName + i;
if (!isExistingName(name)) {
return generatedNames[name] = name;
}
i++;
}
var name = generateUniqueName(baseName, isExistingName);
return generatedNames[name] = name;
}

function assignGeneratedName(node: Node, name: string) {
Expand Down Expand Up @@ -10686,6 +10718,41 @@ module ts {
!hasProperty(getGeneratedNamesForSourceFile(getSourceFile(location)), name);
}

function getBlockScopedVariableId(n: Identifier): number {
Debug.assert(!nodeIsSynthesized(n));

// ignore name parts of property access expressions
if (n.parent.kind === SyntaxKind.PropertyAccessExpression &&
(<PropertyAccessExpression>n.parent).name === n) {
return undefined;
}

// ignore property names in object binding patterns
if (n.parent.kind === SyntaxKind.BindingElement &&
(<BindingElement>n.parent).propertyName === n) {
return undefined;
}

// for names in variable declarations and binding elements try to short circuit and fetch symbol from the node
var declarationSymbol: Symbol =
(n.parent.kind === SyntaxKind.VariableDeclaration && (<VariableDeclaration>n.parent).name === n) ||
n.parent.kind === SyntaxKind.BindingElement
? getSymbolOfNode(n.parent)
: undefined;

var symbol = declarationSymbol ||
getNodeLinks(n).resolvedSymbol ||
resolveName(n, n.text, SymbolFlags.BlockScopedVariable | SymbolFlags.Import, /*nodeNotFoundMessage*/ undefined, /*nameArg*/ undefined);

if (symbol && (symbol.flags & SymbolFlags.BlockScopedVariable)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Exclude catch variables

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

// side-effect of calling this method:
// assign id to symbol if it was not yet set
getSymbolLinks(symbol);
return symbol.id;
}
return undefined;
}

function createResolver(): EmitResolver {
return {
getGeneratedNameForNode,
Expand All @@ -10702,6 +10769,7 @@ module ts {
isEntityNameVisible,
getConstantValue,
isUnknownIdentifier,
getBlockScopedVariableId,
};
}

Expand Down Expand Up @@ -11443,7 +11511,8 @@ module ts {
if (isBindingPattern(node.name) && !isBindingPattern(node.parent)) {
return grammarErrorOnNode(node, Diagnostics.A_destructuring_declaration_must_have_an_initializer);
}
if (isConst(node)) {
// const declarations should not be initialized in for-in for-of statements
if (isConst(node) && node.parent.parent.kind !== SyntaxKind.ForInStatement && node.parent.parent.kind !== SyntaxKind.ForOfStatement) {
return grammarErrorOnNode(node, Diagnostics.const_declarations_must_be_initialized);
}
}
Expand Down Expand Up @@ -11485,15 +11554,6 @@ module ts {
if (!declarationList.declarations.length) {
return grammarErrorAtPos(getSourceFileOfNode(declarationList), declarations.pos, declarations.end - declarations.pos, Diagnostics.Variable_declaration_list_cannot_be_empty);
}

if (languageVersion < ScriptTarget.ES6) {
if (isLet(declarationList)) {
return grammarErrorOnFirstToken(declarationList, Diagnostics.let_declarations_are_only_available_when_targeting_ECMAScript_6_and_higher);
}
else if (isConst(declarationList)) {
return grammarErrorOnFirstToken(declarationList, Diagnostics.const_declarations_are_only_available_when_targeting_ECMAScript_6_and_higher);
}
}
}

function allowLetAndConstDeclarations(parent: Node): boolean {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/diagnosticInformationMap.generated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ module ts {
Parameter_0_of_exported_function_has_or_is_using_name_1_from_private_module_2: { code: 4077, category: DiagnosticCategory.Error, key: "Parameter '{0}' of exported function has or is using name '{1}' from private module '{2}'." },
Parameter_0_of_exported_function_has_or_is_using_private_name_1: { code: 4078, category: DiagnosticCategory.Error, key: "Parameter '{0}' of exported function has or is using private name '{1}'." },
Exported_type_alias_0_has_or_is_using_private_name_1: { code: 4081, category: DiagnosticCategory.Error, key: "Exported type alias '{0}' has or is using private name '{1}'." },
Code_in_the_loop_captures_block_scoped_variable_0_in_closure_This_is_natively_supported_in_ECMAScript_6_or_higher: { code: 4091, category: DiagnosticCategory.Error, key: "Code in the loop captures block-scoped variable '{0}' in closure. This is natively supported in ECMAScript 6 or higher." },
The_current_host_does_not_support_the_0_option: { code: 5001, category: DiagnosticCategory.Error, key: "The current host does not support the '{0}' option." },
Cannot_find_the_common_subdirectory_path_for_the_input_files: { code: 5009, category: DiagnosticCategory.Error, key: "Cannot find the common subdirectory path for the input files." },
Cannot_read_file_0_Colon_1: { code: 5012, category: DiagnosticCategory.Error, key: "Cannot read file '{0}': {1}" },
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1581,6 +1581,10 @@
"category": "Error",
"code": 4081
},
"Code in the loop captures block-scoped variable '{0}' in closure. This is natively supported in ECMAScript 6 or higher.": {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the message is confusing. I'll try to think of a good phrasing

Copy link
Contributor

Choose a reason for hiding this comment

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

First, change natively to only

Copy link
Contributor

Choose a reason for hiding this comment

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

Loop contains block scoped variable referenced by a function in the loop.

"category": "Error",
"code": 4091
},
"The current host does not support the '{0}' option.": {
"category": "Error",
"code": 5001
Expand Down
Loading