From 318aa8ce7a47c6b0dd5c12b40f3d219c6d7f4da4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 9 Feb 2015 14:07:09 -0800 Subject: [PATCH 1/6] Don't use dynamic type checks while incrementally parsing. --- src/compiler/parser.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 4ab8d9d1d5..d58a9f1510 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -371,8 +371,8 @@ module ts { return false; } - function moveElementEntirelyPastChangeRange(element: IncrementalElement, delta: number, oldText: string, newText: string, aggressiveChecks: boolean) { - if (element.length) { + function moveElementEntirelyPastChangeRange(element: IncrementalElement, isArray: boolean, delta: number, oldText: string, newText: string, aggressiveChecks: boolean) { + if (isArray) { visitArray(element); } else { @@ -511,7 +511,7 @@ module ts { if (child.pos > changeRangeOldEnd) { // Node is entirely past the change range. We need to move both its pos and // end, forward or backward appropriately. - moveElementEntirelyPastChangeRange(child, delta, oldText, newText, aggressiveChecks); + moveElementEntirelyPastChangeRange(child, /*isArray:*/ false, delta, oldText, newText, aggressiveChecks); return; } @@ -537,7 +537,7 @@ module ts { if (array.pos > changeRangeOldEnd) { // Array is entirely after the change range. We need to move it, and move any of // its children. - moveElementEntirelyPastChangeRange(array, delta, oldText, newText, aggressiveChecks); + moveElementEntirelyPastChangeRange(array, /*isArray:*/ true, delta, oldText, newText, aggressiveChecks); } else { // Check if the element intersects the change range. If it does, then it is not From d37fdfe213a64105e7ce41797b202a7f81e9d878 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 9 Feb 2015 14:12:32 -0800 Subject: [PATCH 2/6] Add additional asserts. --- src/compiler/parser.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index d58a9f1510..3d512a88e5 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -508,6 +508,7 @@ module ts { return; function visitNode(child: IncrementalNode) { + Debug.assert(child.pos <= child.end); if (child.pos > changeRangeOldEnd) { // Node is entirely past the change range. We need to move both its pos and // end, forward or backward appropriately. @@ -531,9 +532,11 @@ module ts { } // Otherwise, the node is entirely before the change range. No need to do anything with it. + Debug.assert(fullEnd < changeStart); } function visitArray(array: IncrementalNodeArray) { + Debug.assert(array.pos <= array.end); if (array.pos > changeRangeOldEnd) { // Array is entirely after the change range. We need to move it, and move any of // its children. From e417f3016b9c5aae660491b5ae6a7c67d12fa648 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 9 Feb 2015 14:23:55 -0800 Subject: [PATCH 3/6] Add additional asserts, and make code more unified. --- src/compiler/parser.ts | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 3d512a88e5..e99b8e71c5 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -400,6 +400,7 @@ module ts { } function visitArray(array: IncrementalNodeArray) { + array._children = undefined; array.pos += delta; array.end += delta; @@ -412,6 +413,7 @@ module ts { function adjustIntersectingElement(element: IncrementalElement, changeStart: number, changeRangeOldEnd: number, changeRangeNewEnd: number, delta: number) { Debug.assert(element.end >= changeStart, "Adjusting an element that was entirely before the change range"); Debug.assert(element.pos <= changeRangeOldEnd, "Adjusting an element that was entirely after the change range"); + Debug.assert(element.pos <= element.end); // We have an element that intersects the change range in some way. It may have its // start, or its end (or both) in the changed range. We want to adjust any part @@ -522,6 +524,7 @@ module ts { var fullEnd = child.end; if (fullEnd >= changeStart) { child.intersectsChange = true; + child._children = undefined; // Adjust the pos or end (or both) of the intersecting element accordingly. adjustIntersectingElement(child, changeStart, changeRangeOldEnd, changeRangeNewEnd, delta); @@ -541,25 +544,27 @@ module ts { // Array is entirely after the change range. We need to move it, and move any of // its children. moveElementEntirelyPastChangeRange(array, /*isArray:*/ true, delta, oldText, newText, aggressiveChecks); + return; } - else { - // Check if the element intersects the change range. If it does, then it is not - // reusable. Also, we'll need to recurse to see what constituent portions we may - // be able to use. - var fullEnd = array.end; - if (fullEnd >= changeStart) { - array.intersectsChange = true; - // Adjust the pos or end (or both) of the intersecting array accordingly. - adjustIntersectingElement(array, changeStart, changeRangeOldEnd, changeRangeNewEnd, delta); - for (var i = 0, n = array.length; i < n; i++) { - visitNode(array[i]); - } + // Check if the element intersects the change range. If it does, then it is not + // reusable. Also, we'll need to recurse to see what constituent portions we may + // be able to use. + var fullEnd = array.end; + if (fullEnd >= changeStart) { + array.intersectsChange = true; + array._children = undefined; + + // Adjust the pos or end (or both) of the intersecting array accordingly. + adjustIntersectingElement(array, changeStart, changeRangeOldEnd, changeRangeNewEnd, delta); + for (var i = 0, n = array.length; i < n; i++) { + visitNode(array[i]); } - // else { - // Otherwise, the array is entirely before the change range. No need to do anything with it. - // } + return; } + + // Otherwise, the array is entirely before the change range. No need to do anything with it. + Debug.assert(fullEnd < changeStart); } } From a79e8e928be3a4847508959799acfc3a47b2b70a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 9 Feb 2015 14:34:47 -0800 Subject: [PATCH 4/6] Remove code duplication in isModuleElement. --- src/compiler/parser.ts | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index e99b8e71c5..8f7f68d9b6 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -1657,8 +1657,8 @@ module ts { return result; } - function parseListElement(kind: ParsingContext, parseElement: () => T): T { - var node = currentNode(kind); + function parseListElement(parsingContext: ParsingContext, parseElement: () => T): T { + var node = currentNode(parsingContext); if (node) { return consumeNode(node); } @@ -1815,29 +1815,10 @@ module ts { case SyntaxKind.InterfaceDeclaration: case SyntaxKind.ModuleDeclaration: case SyntaxKind.EnumDeclaration: - - // Keep in sync with isStatement: - case SyntaxKind.FunctionDeclaration: - case SyntaxKind.VariableStatement: - case SyntaxKind.Block: - case SyntaxKind.IfStatement: - case SyntaxKind.ExpressionStatement: - case SyntaxKind.ThrowStatement: - case SyntaxKind.ReturnStatement: - case SyntaxKind.SwitchStatement: - case SyntaxKind.BreakStatement: - case SyntaxKind.ContinueStatement: - case SyntaxKind.ForInStatement: - case SyntaxKind.ForStatement: - case SyntaxKind.WhileStatement: - case SyntaxKind.WithStatement: - case SyntaxKind.EmptyStatement: - case SyntaxKind.TryStatement: - case SyntaxKind.LabeledStatement: - case SyntaxKind.DoStatement: - case SyntaxKind.DebuggerStatement: return true; } + + return isReusableStatement(node); } return false; From 17dd6c2de01b4ded0d7afeec39d19f13c0e858a2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 9 Feb 2015 14:40:03 -0800 Subject: [PATCH 5/6] Be more conservative about reusing parameters. --- src/compiler/parser.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 8f7f68d9b6..cd0d86bed4 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -1924,9 +1924,13 @@ module ts { } function isReusableParameter(node: Node) { - // TODO: this most likely needs the same initializer check that - // isReusableVariableDeclaration has. - return node.kind === SyntaxKind.Parameter; + if (node.kind !== SyntaxKind.Parameter) { + return false; + } + + // See the comment in isReusableVariableDeclaration for why we do this. + var parameter = node; + return parameter.initializer === undefined; } // Returns true if we should abort parsing. From 2eb1a213c724bcf429134d09795f8d2bfcf724e6 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 9 Feb 2015 14:55:54 -0800 Subject: [PATCH 6/6] Prevent index out of bounds exception. --- src/compiler/parser.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index cd0d86bed4..a1dc0ef7b1 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -850,7 +850,7 @@ module ts { // Much of the time the parser will need the very next node in the array that // we just returned a node from.So just simply check for that case and move // forward in the array instead of searching for the node again. - if (current && current.end === position && currentArrayIndex < currentArray.length) { + if (current && current.end === position && currentArrayIndex < (currentArray.length - 1)) { currentArrayIndex++; current = currentArray[currentArrayIndex]; } @@ -886,6 +886,7 @@ module ts { // Recurse into the source file to find the highest node at this position. forEachChild(sourceFile, visitNode, visitArray); + return; function visitNode(node: Node) { if (position >= node.pos && position < node.end) {