From 8820ca059683d4aba69785288fc9080c64c68f8a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 12 Dec 2014 03:25:49 -0800 Subject: [PATCH] Change the error-bit to be a node-flag and not a parser context flag. Do not reuse nodes with errors in them. We need to reparse them to make sure we produce the right errors the second time around. --- src/compiler/parser.ts | 10 ++++- src/compiler/types.ts | 46 +++++++++++----------- src/compiler/utilities.ts | 10 ++--- tests/cases/unittests/incrementalParser.ts | 11 ++++-- 4 files changed, 45 insertions(+), 32 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index ffe23c9d36d..37565a89843 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -1335,7 +1335,7 @@ module ts { // flag so that we don't mark any subsequent nodes. if (parseErrorBeforeNextFinishedNode) { parseErrorBeforeNextFinishedNode = false; - node.parserContextFlags |= ParserContextFlags.ContainsError; + node.flags |= NodeFlags.ContainsError; } return node; @@ -1684,6 +1684,12 @@ module ts { return undefined; } + // Can't reuse a node that contains a parse error. This is necessary so that we + // produce the same set of errors again. + if (containsParseError(node)) { + return undefined; + } + // We can only reuse a node if it was parsed under the same strict mode that we're // currently in. i.e. if we originally parsed a node in non-strict mode, but then // the user added 'using strict' at the top of the file, then we can't use that node @@ -1695,7 +1701,7 @@ module ts { // differently depending on what mode it is in. // // This also applies to all our other context flags as well. - var nodeContextFlags = node.parserContextFlags || 0; + var nodeContextFlags = node.parserContextFlags & ParserContextFlags.FlagsMask; if (nodeContextFlags !== contextFlags) { return undefined; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index ee9694e0568..e7d215890d9 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -281,18 +281,29 @@ module ts { } export const enum NodeFlags { - Export = 0x00000001, // Declarations - Ambient = 0x00000002, // Declarations - Public = 0x00000010, // Property/Method - Private = 0x00000020, // Property/Method - Protected = 0x00000040, // Property/Method - Static = 0x00000080, // Property/Method - MultiLine = 0x00000100, // Multi-line array or object literal - Synthetic = 0x00000200, // Synthetic node (for full fidelity) - DeclarationFile = 0x00000400, // Node is a .d.ts file - Let = 0x00000800, // Variable declaration - Const = 0x00001000, // Variable declaration - OctalLiteral = 0x00002000, + Export = 0x00000001, // Declarations + Ambient = 0x00000002, // Declarations + Public = 0x00000010, // Property/Method + Private = 0x00000020, // Property/Method + Protected = 0x00000040, // Property/Method + Static = 0x00000080, // Property/Method + MultiLine = 0x00000100, // Multi-line array or object literal + Synthetic = 0x00000200, // Synthetic node (for full fidelity) + DeclarationFile = 0x00000400, // Node is a .d.ts file + Let = 0x00000800, // Variable declaration + Const = 0x00001000, // Variable declaration + OctalLiteral = 0x00002000, + + // If the parser encountered an error when parsing the code that created this node. Note + // the parser only sets this directly on the node it creates right after encountering the + // error. We then propagate that flag upwards to parent nodes during incremental parsing. + ContainsError = 0x00004000, + + // Used during incremental parsing to determine if we need to visit this node to see if + // any of its children had an error. Once we compute that once, we can set this bit on the + // node to know that we never have to do it again. From that point on, we can just check + // the node directly for 'ContainsError'. + HasPropagatedChildContainsErrorFlag = 0x00008000, Modifier = Export | Ambient | Public | Private | Protected | Static, AccessibilityModifier = Public | Private | Protected, @@ -313,16 +324,7 @@ module ts { // If this node was parsed in the parameters of a generator. GeneratorParameter = 1 << 3, - // If the parser encountered an error when parsing the code that created this node. Note - // the parser only sets this directly on the node it creates right after encountering the - // error. We then propagate that flag upwards to parent nodes during incremental parsing. - ContainsError = 1 << 4, - - // Used during incremental parsing to determine if we need to visit this node to see if - // any of its children had an error. Once we compute that once, we can set this bit on the - // node to know that we never have to do it again. From that point on, we can just check - // the node directly for 'ContainsError'. - HasPropagatedChildContainsErrorFlag = 1 << 5 + FlagsMask = StrictMode | DisallowIn | Yield | GeneratorParameter, } export interface Node extends TextRange { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 62d598ea2fc..d99127c9483 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -68,25 +68,25 @@ module ts { // Returns true if this node contains a parse error anywhere underneath it. export function containsParseError(node: Node): boolean { - if (!hasFlag(node.parserContextFlags, ParserContextFlags.HasPropagatedChildContainsErrorFlag)) { + if (!hasFlag(node.flags, NodeFlags.HasPropagatedChildContainsErrorFlag)) { // A node is considered to contain a parse error if: // a) the parser explicitly marked that it had an error // b) any of it's children reported that it had an error. - var val = hasFlag(node.parserContextFlags, ParserContextFlags.ContainsError) || + var val = hasFlag(node.flags, NodeFlags.ContainsError) || forEachChild(node, containsParseError); // If so, mark ourselves accordingly. if (val) { - node.parserContextFlags |= ParserContextFlags.ContainsError; + node.flags |= NodeFlags.ContainsError; } // Also mark that we've propogated the child information to this node. This way we can // always consult the bit directly on this node without needing to check its children // again. - node.parserContextFlags |= ParserContextFlags.HasPropagatedChildContainsErrorFlag; + node.flags |= NodeFlags.HasPropagatedChildContainsErrorFlag; } - return hasFlag(node.parserContextFlags, ParserContextFlags.ContainsError); + return hasFlag(node.flags, NodeFlags.ContainsError); } export function getSourceFileOfNode(node: Node): SourceFile { diff --git a/tests/cases/unittests/incrementalParser.ts b/tests/cases/unittests/incrementalParser.ts index 8a57556363d..282f126344b 100644 --- a/tests/cases/unittests/incrementalParser.ts +++ b/tests/cases/unittests/incrementalParser.ts @@ -88,6 +88,11 @@ module ts { assert.equal(node1.pos, node2.pos, "node1.pos !== node2.pos"); assert.equal(node1.end, node2.end, "node1.end !== node2.end"); assert.equal(node1.kind, node2.kind, "node1.kind !== node2.kind"); + + // call this on both nodes to ensure all propagated flags have been set (and thus can be + // compared). + ts.containsParseError(node1); + ts.containsParseError(node2); assert.equal(node1.flags, node2.flags, "node1.flags !== node2.flags"); assert.equal(node1.parserContextFlags, node2.parserContextFlags, "node1.parserContextFlags !== node2.parserContextFlags"); @@ -177,7 +182,6 @@ module ts { } describe('Incremental',() => { - debugger; it('Inserting into method',() => { debugger; var source = "class C {\r\n" + @@ -768,12 +772,13 @@ module m3 { }\ }); it('Moving methods from object literal to class',() => { + debugger; var source = "var v = { public A() { } public B() { } public C() { } }" var oldText = ScriptSnapshot.fromString(source); var newTextAndChange = withChange(oldText, 0, "var v =".length, "class C"); - compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 0); + compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 4); }); it('Moving index signatures from class to interface',() => { @@ -809,7 +814,7 @@ module m3 { }\ var oldText = ScriptSnapshot.fromString(source); var newTextAndChange = withChange(oldText, 0, "var v =".length, "class C"); - compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 16); + compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 4); }); // Simulated typing tests.