Skip to content

Commit 3b3a94c

Browse files
vladimavladima
committed
addressed PR feedback
1 parent 09d5582 commit 3b3a94c

File tree

7 files changed

+49
-78
lines changed

7 files changed

+49
-78
lines changed

‎src/compiler/binder.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -244,10 +244,13 @@ module ts {
244244
}
245245

246246
if (isBlockScopeContainer) {
247-
// clean locals in block scope container if
248-
// - current node does not have local variables
249-
// - current node is not source file (source file always have locals)
250-
setBlockScopeContainer(node, /*cleanLocals*/ (symbolKind & SymbolFlags.HasLocals) == 0 && node.kind !== SyntaxKind.SourceFile);
247+
// in incremental scenarios we might reuse nodes that already have locals being allocated
248+
// during the bind step these locals should be dropped to prevent using stale data.
249+
// locals should always be dropped unless they were previously initialized by the binder
250+
// these cases are:
251+
// - node has locals (symbolKind & HasLocals) !== 0
252+
// - node is a source file
253+
setBlockScopeContainer(node, /*cleanLocals*/ (symbolKind & SymbolFlags.HasLocals) === 0 && node.kind !== SyntaxKind.SourceFile);
251254
}
252255

253256
forEachChild(node, bind);
@@ -354,7 +357,6 @@ module ts {
354357

355358
function bindCatchVariableDeclaration(node: CatchClause) {
356359
bindChildren(node, /*symbolKind:*/ 0, /*isBlockScopeContainer:*/ true);
357-
358360
}
359361

360362
function bindBlockScopedVariableDeclaration(node: Declaration) {

‎src/compiler/checker.ts

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5080,6 +5080,18 @@ module ts {
50805080
return getNarrowedTypeOfSymbol(getExportSymbolOfValueSymbolIfExported(symbol), node);
50815081
}
50825082

5083+
function isInsideFunction(node: Node, threshold: Node): boolean {
5084+
var current = node;
5085+
while (current && current !== threshold) {
5086+
if (isAnyFunction(current)) {
5087+
return true;
5088+
}
5089+
current = current.parent;
5090+
}
5091+
5092+
return false;
5093+
}
5094+
50835095
function checkBlockScopedBindingCapturedInLoop(node: Identifier, symbol: Symbol): void {
50845096
if (languageVersion >= ScriptTarget.ES6 ||
50855097
(symbol.flags & SymbolFlags.BlockScopedVariable) === 0 ||
@@ -5104,21 +5116,13 @@ module ts {
51045116
container = container.parent;
51055117
}
51065118

5107-
var inFunction = false;
5108-
var current = node.parent;
5109-
while (current && current !== container) {
5110-
if (isAnyFunction(current)) {
5111-
inFunction = true;
5112-
break;
5113-
}
5114-
current = current.parent;
5115-
}
5119+
var inFunction = isInsideFunction(node.parent, container);
51165120

5117-
var current: Node = container;
5121+
var current = container;
51185122
while (current && !nodeStartsNewLexicalEnvironment(current)) {
51195123
if (isIterationStatement(current, /*lookInLabeledStatements*/ false)) {
51205124
if (inFunction) {
5121-
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));
5125+
grammarErrorOnFirstToken(current, Diagnostics.Loop_contains_block_scoped_variable_0_referenced_by_a_function_in_the_loop_This_is_only_supported_in_ECMAScript_6_or_higher, declarationNameToString(node));
51225126
}
51235127
// mark value declaration so during emit they can have a special handling
51245128
getNodeLinks(<VariableDeclaration>symbol.valueDeclaration).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
@@ -10744,7 +10748,12 @@ module ts {
1074410748
getNodeLinks(n).resolvedSymbol ||
1074510749
resolveName(n, n.text, SymbolFlags.BlockScopedVariable | SymbolFlags.Import, /*nodeNotFoundMessage*/ undefined, /*nameArg*/ undefined);
1074610750

10747-
if (symbol && (symbol.flags & SymbolFlags.BlockScopedVariable)) {
10751+
var isLetOrConst =
10752+
symbol &&
10753+
(symbol.flags & SymbolFlags.BlockScopedVariable) &&
10754+
symbol.valueDeclaration.parent.kind !== SyntaxKind.CatchClause;
10755+
10756+
if (isLetOrConst) {
1074810757
// side-effect of calling this method:
1074910758
// assign id to symbol if it was not yet set
1075010759
getSymbolLinks(symbol);

‎src/compiler/diagnosticInformationMap.generated.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ module ts {
397397
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}'." },
398398
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}'." },
399399
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}'." },
400-
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." },
400+
Loop_contains_block_scoped_variable_0_referenced_by_a_function_in_the_loop_This_is_only_supported_in_ECMAScript_6_or_higher: { code: 4091, category: DiagnosticCategory.Error, key: "Loop contains block-scoped variable '{0}' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher." },
401401
The_current_host_does_not_support_the_0_option: { code: 5001, category: DiagnosticCategory.Error, key: "The current host does not support the '{0}' option." },
402402
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." },
403403
Cannot_read_file_0_Colon_1: { code: 5012, category: DiagnosticCategory.Error, key: "Cannot read file '{0}': {1}" },

‎src/compiler/diagnosticMessages.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1581,7 +1581,7 @@
15811581
"category": "Error",
15821582
"code": 4081
15831583
},
1584-
"Code in the loop captures block-scoped variable '{0}' in closure. This is natively supported in ECMAScript 6 or higher.": {
1584+
"Loop contains block-scoped variable '{0}' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.": {
15851585
"category": "Error",
15861586
"code": 4091
15871587
},

‎src/compiler/emitter.ts

Lines changed: 4 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,6 @@ module ts {
3434
previous: ScopeFrame;
3535
}
3636

37-
// isExisingName function has signature string -> boolean, however check if name is unique should be performed
38-
// in the context of some location. Instead of creating function expression and closing over location
39-
// every time isExisingName is called we use one single instance of NameLookup that is effectively a
40-
// handrolled closure where value of location can be swapped. This allows to avoid allocations of closures on
41-
// every call and use one shared instance instead
42-
interface NameLookup {
43-
setLocation(location: Node): void;
44-
isExistingName(name: string): boolean;
45-
}
46-
4737
type GetSymbolAccessibilityDiagnostic = (symbolAccesibilityResult: SymbolAccessiblityResult) => SymbolAccessibilityDiagnostic;
4838

4939
interface EmitTextWriterWithSymbolWriter extends EmitTextWriter, SymbolWriter {
@@ -1541,7 +1531,6 @@ module ts {
15411531
var sourceMapDataList: SourceMapData[] = compilerOptions.sourceMap ? [] : undefined;
15421532
var diagnostics: Diagnostic[] = [];
15431533
var newLine = host.getNewLine();
1544-
var nameLookup: NameLookup;
15451534

15461535
if (targetSourceFile === undefined) {
15471536
forEach(host.getSourceFiles(), sourceFile => {
@@ -1686,38 +1675,14 @@ module ts {
16861675
}
16871676
}
16881677

1689-
// creates instance of NameLookup to be used in 'isExisingName' checks.
1690-
// see comment for NameLookup for more information
1691-
function createNameLookup(): NameLookup {
1692-
var location: Node;
1693-
return {
1694-
setLocation,
1695-
isExistingName: checkName
1696-
}
1697-
1698-
function setLocation(l: Node): void {
1699-
location = l;
1700-
}
1701-
1702-
function checkName(name: string): boolean {
1703-
Debug.assert(location !== undefined);
1704-
return isExistingName(location, name);
1705-
}
1706-
}
1707-
1708-
function makeUniqueName(location: Node, baseName: string): string {
1678+
function generateUniqueNameForLocation(location: Node, baseName: string): string {
17091679
var name: string
17101680
// first try to check if base name can be used as is
17111681
if (!isExistingName(location, baseName)) {
17121682
name = baseName;
17131683
}
17141684
else {
1715-
if (!nameLookup) {
1716-
nameLookup = createNameLookup();
1717-
}
1718-
nameLookup.setLocation(location);
1719-
name = generateUniqueName(baseName, nameLookup.isExistingName);
1720-
nameLookup.setLocation(undefined);
1685+
name = generateUniqueName(baseName, n => isExistingName(location, n));
17211686
}
17221687

17231688
if (!currentScopeNames) {
@@ -2560,7 +2525,7 @@ module ts {
25602525

25612526
function getBlockScopedVariableId(node: Identifier): number {
25622527
// return undefined for synthesized nodes
2563-
return node.parent && resolver.getBlockScopedVariableId(node);
2528+
return !nodeIsSynthesized(node) && resolver.getBlockScopedVariableId(node);
25642529
}
25652530

25662531
function emitIdentifier(node: Identifier) {
@@ -3924,7 +3889,7 @@ module ts {
39243889
? blockScopeContainer
39253890
: blockScopeContainer.parent;
39263891

3927-
var generatedName = makeUniqueName(parent, (<Identifier>node).text);
3892+
var generatedName = generateUniqueNameForLocation(parent, (<Identifier>node).text);
39283893
var variableId = resolver.getBlockScopedVariableId(<Identifier>node);
39293894
if (!generatedBlockScopeNames) {
39303895
generatedBlockScopeNames = [];

‎src/compiler/utilities.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,17 +1139,14 @@ module ts {
11391139
return createTextChangeRange(createTextSpanFromBounds(oldStartN, oldEndN), /*newLength: */newEndN - oldStartN);
11401140
}
11411141

1142-
// @internal
11431142
export function nodeStartsNewLexicalEnvironment(n: Node): boolean {
11441143
return isAnyFunction(n) || n.kind === SyntaxKind.ModuleDeclaration || n.kind === SyntaxKind.SourceFile;
11451144
}
11461145

1147-
// @internal
11481146
export function nodeIsSynthesized(node: Node): boolean {
11491147
return node.pos === -1 && node.end === -1;
11501148
}
11511149

1152-
// @internal
11531150
export function createSynthesizedNode(kind: SyntaxKind, startsOnNewLine?: boolean): Node {
11541151
var node = <SynthesizedNode>createNode(kind);
11551152
node.pos = -1;
@@ -1158,7 +1155,6 @@ module ts {
11581155
return node;
11591156
}
11601157

1161-
// @internal
11621158
export function generateUniqueName(baseName: string, isExistingName: (name: string) => boolean): string {
11631159
// First try '_name'
11641160
if (baseName.charCodeAt(0) !== CharacterCodes._) {
@@ -1173,15 +1169,14 @@ module ts {
11731169
}
11741170
var i = 1;
11751171
while (true) {
1176-
name = baseName + i;
1172+
var name = baseName + i;
11771173
if (!isExistingName(name)) {
11781174
return name;
11791175
}
11801176
i++;
11811177
}
11821178
}
11831179

1184-
// @internal
11851180
export function createDiagnosticCollection(): DiagnosticCollection {
11861181
var nonFileDiagnostics: Diagnostic[] = [];
11871182
var fileDiagnostics: Map<Diagnostic[]> = {};

‎tests/baselines/reference/downlevelLetConst18.errors.txt

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,60 +1,60 @@
1-
tests/cases/compiler/downlevelLetConst18.ts(3,1): error TS4091: Code in the loop captures block-scoped variable 'x' in closure. This is natively supported in ECMAScript 6 or higher.
1+
tests/cases/compiler/downlevelLetConst18.ts(3,1): error TS4091: Loop contains block-scoped variable 'x' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.
22
tests/cases/compiler/downlevelLetConst18.ts(4,14): error TS2393: Duplicate function implementation.
3-
tests/cases/compiler/downlevelLetConst18.ts(7,1): error TS4091: Code in the loop captures block-scoped variable 'x' in closure. This is natively supported in ECMAScript 6 or higher.
3+
tests/cases/compiler/downlevelLetConst18.ts(7,1): error TS4091: Loop contains block-scoped variable 'x' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.
44
tests/cases/compiler/downlevelLetConst18.ts(8,14): error TS2393: Duplicate function implementation.
5-
tests/cases/compiler/downlevelLetConst18.ts(11,1): error TS4091: Code in the loop captures block-scoped variable 'x' in closure. This is natively supported in ECMAScript 6 or higher.
6-
tests/cases/compiler/downlevelLetConst18.ts(15,1): error TS4091: Code in the loop captures block-scoped variable 'x' in closure. This is natively supported in ECMAScript 6 or higher.
7-
tests/cases/compiler/downlevelLetConst18.ts(19,1): error TS4091: Code in the loop captures block-scoped variable 'x' in closure. This is natively supported in ECMAScript 6 or higher.
8-
tests/cases/compiler/downlevelLetConst18.ts(23,1): error TS4091: Code in the loop captures block-scoped variable 'x' in closure. This is natively supported in ECMAScript 6 or higher.
9-
tests/cases/compiler/downlevelLetConst18.ts(27,1): error TS4091: Code in the loop captures block-scoped variable 'x' in closure. This is natively supported in ECMAScript 6 or higher.
5+
tests/cases/compiler/downlevelLetConst18.ts(11,1): error TS4091: Loop contains block-scoped variable 'x' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.
6+
tests/cases/compiler/downlevelLetConst18.ts(15,1): error TS4091: Loop contains block-scoped variable 'x' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.
7+
tests/cases/compiler/downlevelLetConst18.ts(19,1): error TS4091: Loop contains block-scoped variable 'x' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.
8+
tests/cases/compiler/downlevelLetConst18.ts(23,1): error TS4091: Loop contains block-scoped variable 'x' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.
9+
tests/cases/compiler/downlevelLetConst18.ts(27,1): error TS4091: Loop contains block-scoped variable 'x' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.
1010

1111

1212
==== tests/cases/compiler/downlevelLetConst18.ts (9 errors) ====
1313
'use strict'
1414

1515
for (let x; ;) {
1616
~~~
17-
!!! error TS4091: Code in the loop captures block-scoped variable 'x' in closure. This is natively supported in ECMAScript 6 or higher.
17+
!!! error TS4091: Loop contains block-scoped variable 'x' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.
1818
function foo() { x };
1919
~~~
2020
!!! error TS2393: Duplicate function implementation.
2121
}
2222

2323
for (let x; ;) {
2424
~~~
25-
!!! error TS4091: Code in the loop captures block-scoped variable 'x' in closure. This is natively supported in ECMAScript 6 or higher.
25+
!!! error TS4091: Loop contains block-scoped variable 'x' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.
2626
function foo() { x };
2727
~~~
2828
!!! error TS2393: Duplicate function implementation.
2929
}
3030

3131
for (let x; ;) {
3232
~~~
33-
!!! error TS4091: Code in the loop captures block-scoped variable 'x' in closure. This is natively supported in ECMAScript 6 or higher.
33+
!!! error TS4091: Loop contains block-scoped variable 'x' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.
3434
(() => { x })();
3535
}
3636

3737
for (const x = 1; ;) {
3838
~~~
39-
!!! error TS4091: Code in the loop captures block-scoped variable 'x' in closure. This is natively supported in ECMAScript 6 or higher.
39+
!!! error TS4091: Loop contains block-scoped variable 'x' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.
4040
(() => { x })();
4141
}
4242

4343
for (let x; ;) {
4444
~~~
45-
!!! error TS4091: Code in the loop captures block-scoped variable 'x' in closure. This is natively supported in ECMAScript 6 or higher.
45+
!!! error TS4091: Loop contains block-scoped variable 'x' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.
4646
({ foo() { x }})
4747
}
4848

4949
for (let x; ;) {
5050
~~~
51-
!!! error TS4091: Code in the loop captures block-scoped variable 'x' in closure. This is natively supported in ECMAScript 6 or higher.
51+
!!! error TS4091: Loop contains block-scoped variable 'x' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.
5252
({ get foo() { return x } })
5353
}
5454

5555
for (let x; ;) {
5656
~~~
57-
!!! error TS4091: Code in the loop captures block-scoped variable 'x' in closure. This is natively supported in ECMAScript 6 or higher.
57+
!!! error TS4091: Loop contains block-scoped variable 'x' referenced by a function in the loop. This is only supported in ECMAScript 6 or higher.
5858
({ set foo(v) { x } })
5959
}
6060

0 commit comments

Comments
 (0)