From beb1aa3d0a01291388d8565d1a60f2be281be513 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Tue, 15 Sep 2015 10:36:55 -0700 Subject: [PATCH] addressed PR feedback --- src/compiler/binder.ts | 55 +++++++++++------- .../reference/reachabilityChecks1.errors.txt | 43 +++++++++++--- .../reference/reachabilityChecks1.js | 56 +++++++++++++++++++ .../reference/reachabilityChecks2.errors.txt | 18 ++++++ .../reference/reachabilityChecks2.js | 12 ++++ .../reference/reachabilityChecks2.symbols | 8 --- .../reference/reachabilityChecks2.types | 10 ---- tests/cases/compiler/reachabilityChecks1.ts | 26 +++++++++ tests/cases/compiler/reachabilityChecks2.ts | 7 +++ 9 files changed, 191 insertions(+), 44 deletions(-) create mode 100644 tests/baselines/reference/reachabilityChecks2.errors.txt delete mode 100644 tests/baselines/reference/reachabilityChecks2.symbols delete mode 100644 tests/baselines/reference/reachabilityChecks2.types diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index c29c761c28a..cebc930d328 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -108,7 +108,7 @@ namespace ts { let hasExplicitReturn: boolean; let currentReachabilityState: Reachability; let labelStack: Reachability[]; - let labels: Map; + let labelIndexMap: Map; let implicitLabels: number[]; // If this file is an external module, then it is automatically in strict-mode according to @@ -373,13 +373,13 @@ namespace ts { if (saveState) { savedReachabilityState = currentReachabilityState; savedLabelStack = labelStack; - savedLabels = labels; + savedLabels = labelIndexMap; savedImplicitLabels = implicitLabels; savedHasExplicitReturn = hasExplicitReturn; currentReachabilityState = Reachability.Reachable; hasExplicitReturn = false; - labelStack = labels = implicitLabels = undefined; + labelStack = labelIndexMap = implicitLabels = undefined; } if (!bindReachableStatement(node)) { @@ -397,17 +397,21 @@ namespace ts { hasExplicitReturn = savedHasExplicitReturn; currentReachabilityState = savedReachabilityState; labelStack = savedLabelStack; - labels = savedLabels; + labelIndexMap = savedLabels; implicitLabels = savedImplicitLabels; } } + /** + * Returns true if node and its subnodes were successfully traversed. + * Returning false means that node was not examined and caller needs to dive into the node himself. + */ function bindReachableStatement(n: Node): boolean { if (checkUnreachable(n)) { return false; } - switch(n.kind) { + switch (n.kind) { case SyntaxKind.WhileStatement: return bindWhileStatement(n); case SyntaxKind.DoStatement: @@ -460,7 +464,7 @@ namespace ts { const postDoLabel = pushImplicitLabel(); bind(n.statement); - var postDoState = n.expression.kind === SyntaxKind.TrueKeyword ? Reachability.Unreachable : preDoState; + const postDoState = n.expression.kind === SyntaxKind.TrueKeyword ? Reachability.Unreachable : preDoState; popImplicitLabel(postDoLabel, postDoState); // bind expressions (don't affect reachability) @@ -505,7 +509,11 @@ namespace ts { } function bindIfStatement(n: IfStatement): boolean { + // denotes reachability state when entering 'thenStatement' part of the if statement: + // i.e. if condition is false then thenStatement is unreachable const ifTrueState = n.expression.kind === SyntaxKind.FalseKeyword ? Reachability.Unreachable : currentReachabilityState; + // denotes reachability state when entering 'elseStatement': + // i.e. if condition is true then elseStatement is unreachable const ifFalseState = n.expression.kind === SyntaxKind.TrueKeyword ? Reachability.Unreachable : currentReachabilityState; currentReachabilityState = ifTrueState; @@ -585,7 +593,7 @@ namespace ts { const hasDefault = forEach(n.caseBlock.clauses, c => c.kind === SyntaxKind.DefaultClause); // post switch state is unreachable if switch is exaustive (has a default case ) and does not have fallthrough from the last case - var postSwitchState = hasDefault && currentReachabilityState !== Reachability.Reachable ? Reachability.Unreachable : preSwitchState; + const postSwitchState = hasDefault && currentReachabilityState !== Reachability.Reachable ? Reachability.Unreachable : preSwitchState; popImplicitLabel(postSwitchLabel, postSwitchState); @@ -595,7 +603,7 @@ namespace ts { function bindCaseBlock(n: CaseBlock): boolean { const startState = currentReachabilityState; - for(let clause of n.clauses) { + for (let clause of n.clauses) { currentReachabilityState = startState; bind(clause); if (clause.statements.length && currentReachabilityState === Reachability.Reachable && options.noFallthroughCasesInSwitch) { @@ -610,7 +618,7 @@ namespace ts { // call bind on label (don't affect reachability) bind(n.label); - var ok = pushNamedLabel(n.label); + const ok = pushNamedLabel(n.label); bind(n.statement); if (ok) { popNamedLabel(n.label, currentReachabilityState); @@ -1347,10 +1355,10 @@ namespace ts { function pushNamedLabel(name: Identifier): boolean { initializeReachabilityStateIfNecessary(); - if (hasProperty(labels, name.text)) { + if (hasProperty(labelIndexMap, name.text)) { return false; } - labels[name.text] = labelStack.push(Reachability.Unintialized) - 1; + labelIndexMap[name.text] = labelStack.push(Reachability.Unintialized) - 1; return true; } @@ -1365,11 +1373,11 @@ namespace ts { function popNamedLabel(label: Identifier, outerState: Reachability): void { initializeReachabilityStateIfNecessary(); - let index = labels[label.text]; + let index = labelIndexMap[label.text]; Debug.assert(index !== undefined); Debug.assert(labelStack.length == index + 1); - labels[label.text] = undefined; + labelIndexMap[label.text] = undefined; setCurrentStateAtLabel(labelStack.pop(), outerState, label); } @@ -1400,7 +1408,7 @@ namespace ts { function jumpToLabel(label: Identifier, outerState: Reachability): void { initializeReachabilityStateIfNecessary(); - const index = label ? labels[label.text] : lastOrUndefined(implicitLabels); + const index = label ? labelIndexMap[label.text] : lastOrUndefined(implicitLabels); if (index === undefined) { // reference to unknown label or // break/continue used outside of loops @@ -1411,12 +1419,16 @@ namespace ts { } function checkUnreachable(node: Node): boolean { - switch(currentReachabilityState) { + switch (currentReachabilityState) { case Reachability.Unreachable: const reportError = + // report error on all statements isStatement(node) || + // report error on class declarations node.kind === SyntaxKind.ClassDeclaration || - node.kind === SyntaxKind.ModuleDeclaration || + // report error on instantiated modules or const-enums only modules if preserveConstEnums is set + (node.kind === SyntaxKind.ModuleDeclaration && shouldReportErrorOnModuleDeclaration(node)) || + // report error on regular enums and const enums if preserveConstEnums is set (node.kind === SyntaxKind.EnumDeclaration && (!isConstEnumDeclaration(node) || options.preserveConstEnums)); if (reportError) { @@ -1424,7 +1436,7 @@ namespace ts { // unreachable code is reported if // - user has explicitly asked about it AND - // - statement is in ambient context (statements in ambient context is already an error so we shoult not report extras) AND + // - statement is in not ambient context (statements in ambient context is already an error so we shoult not report extras) AND // - node is not variable statement OR // - node is block scoped variable statement OR // - node is not block scoped variable statement and at least one variable declaration has initializer @@ -1448,14 +1460,19 @@ namespace ts { default: return false; } + + function shouldReportErrorOnModuleDeclaration(node: ModuleDeclaration): boolean { + const instanceState = getModuleInstanceState(node); + return instanceState === ModuleInstanceState.Instantiated || (instanceState === ModuleInstanceState.ConstEnumOnly && options.preserveConstEnums); + } } function initializeReachabilityStateIfNecessary(): void { - if (labels) { + if (labelIndexMap) { return; } currentReachabilityState = Reachability.Reachable; - labels = {}; + labelIndexMap = {}; labelStack = []; implicitLabels = []; } diff --git a/tests/baselines/reference/reachabilityChecks1.errors.txt b/tests/baselines/reference/reachabilityChecks1.errors.txt index ebf1019e7e9..7708867c26c 100644 --- a/tests/baselines/reference/reachabilityChecks1.errors.txt +++ b/tests/baselines/reference/reachabilityChecks1.errors.txt @@ -1,12 +1,13 @@ tests/cases/compiler/reachabilityChecks1.ts(3,1): error TS7027: Unreachable code detected. tests/cases/compiler/reachabilityChecks1.ts(7,5): error TS7027: Unreachable code detected. -tests/cases/compiler/reachabilityChecks1.ts(22,11): error TS7027: Unreachable code detected. -tests/cases/compiler/reachabilityChecks1.ts(28,12): error TS7027: Unreachable code detected. -tests/cases/compiler/reachabilityChecks1.ts(35,10): error TS7027: Unreachable code detected. -tests/cases/compiler/reachabilityChecks1.ts(44,16): error TS7027: Unreachable code detected. +tests/cases/compiler/reachabilityChecks1.ts(19,12): error TS7027: Unreachable code detected. +tests/cases/compiler/reachabilityChecks1.ts(31,12): error TS7027: Unreachable code detected. +tests/cases/compiler/reachabilityChecks1.ts(48,11): error TS7027: Unreachable code detected. +tests/cases/compiler/reachabilityChecks1.ts(61,10): error TS7027: Unreachable code detected. +tests/cases/compiler/reachabilityChecks1.ts(70,16): error TS7027: Unreachable code detected. -==== tests/cases/compiler/reachabilityChecks1.ts (6 errors) ==== +==== tests/cases/compiler/reachabilityChecks1.ts (7 errors) ==== while (true); var x = 1; @@ -20,6 +21,36 @@ tests/cases/compiler/reachabilityChecks1.ts(44,16): error TS7027: Unreachable co !!! error TS7027: Unreachable code detected. } + module A1 { + do {} while(true); + module A { + interface F {} + } + } + + module A2 { + while (true); + module A { + ~ +!!! error TS7027: Unreachable code detected. + var x = 1; + } + } + + module A3 { + while (true); + type T = string; + } + + module A4 { + while (true); + module A { + ~ +!!! error TS7027: Unreachable code detected. + const enum E { X } + } + } + function f1(x) { if (x) { return; @@ -41,8 +72,6 @@ tests/cases/compiler/reachabilityChecks1.ts(44,16): error TS7027: Unreachable co module B { for (; ;); module C { - ~ -!!! error TS7027: Unreachable code detected. } } diff --git a/tests/baselines/reference/reachabilityChecks1.js b/tests/baselines/reference/reachabilityChecks1.js index 3f9ca87422a..dcbfbe89de4 100644 --- a/tests/baselines/reference/reachabilityChecks1.js +++ b/tests/baselines/reference/reachabilityChecks1.js @@ -8,6 +8,32 @@ module A { let x; } +module A1 { + do {} while(true); + module A { + interface F {} + } +} + +module A2 { + while (true); + module A { + var x = 1; + } +} + +module A3 { + while (true); + type T = string; +} + +module A4 { + while (true); + module A { + const enum E { X } + } +} + function f1(x) { if (x) { return; @@ -59,6 +85,36 @@ var A; ; var x; })(A || (A = {})); +var A1; +(function (A1) { + do { } while (true); +})(A1 || (A1 = {})); +var A2; +(function (A2) { + while (true) + ; + var A; + (function (A) { + var x = 1; + })(A || (A = {})); +})(A2 || (A2 = {})); +var A3; +(function (A3) { + while (true) + ; +})(A3 || (A3 = {})); +var A4; +(function (A4) { + while (true) + ; + var A; + (function (A) { + var E; + (function (E) { + E[E["X"] = 0] = "X"; + })(E || (E = {})); + })(A || (A = {})); +})(A4 || (A4 = {})); function f1(x) { if (x) { return; diff --git a/tests/baselines/reference/reachabilityChecks2.errors.txt b/tests/baselines/reference/reachabilityChecks2.errors.txt new file mode 100644 index 00000000000..3de7adc50a6 --- /dev/null +++ b/tests/baselines/reference/reachabilityChecks2.errors.txt @@ -0,0 +1,18 @@ +tests/cases/compiler/reachabilityChecks2.ts(5,8): error TS7027: Unreachable code detected. + + +==== tests/cases/compiler/reachabilityChecks2.ts (1 errors) ==== + + while (true) { } + const enum E { X } + + module A4 { + ~~ +!!! error TS7027: Unreachable code detected. + while (true); + module A { + const enum E { X } + } + } + + \ No newline at end of file diff --git a/tests/baselines/reference/reachabilityChecks2.js b/tests/baselines/reference/reachabilityChecks2.js index 80534c0d7de..a17b51b4ca6 100644 --- a/tests/baselines/reference/reachabilityChecks2.js +++ b/tests/baselines/reference/reachabilityChecks2.js @@ -3,7 +3,19 @@ while (true) { } const enum E { X } +module A4 { + while (true); + module A { + const enum E { X } + } +} + //// [reachabilityChecks2.js] while (true) { } +var A4; +(function (A4) { + while (true) + ; +})(A4 || (A4 = {})); diff --git a/tests/baselines/reference/reachabilityChecks2.symbols b/tests/baselines/reference/reachabilityChecks2.symbols deleted file mode 100644 index d289304dffa..00000000000 --- a/tests/baselines/reference/reachabilityChecks2.symbols +++ /dev/null @@ -1,8 +0,0 @@ -=== tests/cases/compiler/reachabilityChecks2.ts === - -while (true) { } -const enum E { X } ->E : Symbol(E, Decl(reachabilityChecks2.ts, 1, 16)) ->X : Symbol(E.X, Decl(reachabilityChecks2.ts, 2, 14)) - - diff --git a/tests/baselines/reference/reachabilityChecks2.types b/tests/baselines/reference/reachabilityChecks2.types deleted file mode 100644 index 38dad87c6a1..00000000000 --- a/tests/baselines/reference/reachabilityChecks2.types +++ /dev/null @@ -1,10 +0,0 @@ -=== tests/cases/compiler/reachabilityChecks2.ts === - -while (true) { } ->true : boolean - -const enum E { X } ->E : E ->X : E - - diff --git a/tests/cases/compiler/reachabilityChecks1.ts b/tests/cases/compiler/reachabilityChecks1.ts index bc7662486a6..ce009e13456 100644 --- a/tests/cases/compiler/reachabilityChecks1.ts +++ b/tests/cases/compiler/reachabilityChecks1.ts @@ -9,6 +9,32 @@ module A { let x; } +module A1 { + do {} while(true); + module A { + interface F {} + } +} + +module A2 { + while (true); + module A { + var x = 1; + } +} + +module A3 { + while (true); + type T = string; +} + +module A4 { + while (true); + module A { + const enum E { X } + } +} + function f1(x) { if (x) { return; diff --git a/tests/cases/compiler/reachabilityChecks2.ts b/tests/cases/compiler/reachabilityChecks2.ts index fc8d53bde8f..9a2b519ca4c 100644 --- a/tests/cases/compiler/reachabilityChecks2.ts +++ b/tests/cases/compiler/reachabilityChecks2.ts @@ -4,3 +4,10 @@ while (true) { } const enum E { X } +module A4 { + while (true); + module A { + const enum E { X } + } +} +