Skip to content

Commit

Permalink
Reuse subtree transform flags for incrementally parsed nodes (#12088)
Browse files Browse the repository at this point in the history
  • Loading branch information
@rbuckton @mhegazy
rbuckton authored and mhegazy committed Nov 8, 2016
1 parent be2e8e8 commit ddc4ae7
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 71 deletions.
66 changes: 64 additions & 2 deletions src/compiler/binder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,7 @@ namespace ts {
skipTransformFlagAggregation = true;
bindChildrenWorker(node);
skipTransformFlagAggregation = false;
subtreeTransformFlags |= node.transformFlags & ~getTransformFlagsSubtreeExclusions(node.kind);
}
else {
const savedSubtreeTransformFlags = subtreeTransformFlags;
Expand Down Expand Up @@ -895,8 +896,8 @@ namespace ts {
const enclosingLabeledStatement = node.parent.kind === SyntaxKind.LabeledStatement
? lastOrUndefined(activeLabels)
: undefined;
// if do statement is wrapped in labeled statement then target labels for break/continue with or without
// label should be the same
// if do statement is wrapped in labeled statement then target labels for break/continue with or without
// label should be the same
const preConditionLabel = enclosingLabeledStatement ? enclosingLabeledStatement.continueTarget : createBranchLabel();
const postDoLabel = enclosingLabeledStatement ? enclosingLabeledStatement.breakTarget : createBranchLabel();
addAntecedent(preDoLabel, currentFlow);
Expand Down Expand Up @@ -3206,4 +3207,65 @@ namespace ts {
node.transformFlags = transformFlags | TransformFlags.HasComputedFlags;
return transformFlags & ~excludeFlags;
}

/**
* Gets the transform flags to exclude when unioning the transform flags of a subtree.
*
* NOTE: This needs to be kept up-to-date with the exclusions used in `computeTransformFlagsForNode`.
* For performance reasons, `computeTransformFlagsForNode` uses local constant values rather
* than calling this function.
*/
/* @internal */
export function getTransformFlagsSubtreeExclusions(kind: SyntaxKind) {
if (kind >= SyntaxKind.FirstTypeNode && kind <= SyntaxKind.LastTypeNode) {
return TransformFlags.TypeExcludes;
}

switch (kind) {
case SyntaxKind.CallExpression:
case SyntaxKind.NewExpression:
case SyntaxKind.ArrayLiteralExpression:
return TransformFlags.ArrayLiteralOrCallOrNewExcludes;
case SyntaxKind.ModuleDeclaration:
return TransformFlags.ModuleExcludes;
case SyntaxKind.Parameter:
return TransformFlags.ParameterExcludes;
case SyntaxKind.ArrowFunction:
return TransformFlags.ArrowFunctionExcludes;
case SyntaxKind.FunctionExpression:
case SyntaxKind.FunctionDeclaration:
return TransformFlags.FunctionExcludes;
case SyntaxKind.VariableDeclarationList:
return TransformFlags.VariableDeclarationListExcludes;
case SyntaxKind.ClassDeclaration:
case SyntaxKind.ClassExpression:
return TransformFlags.ClassExcludes;
case SyntaxKind.Constructor:
return TransformFlags.ConstructorExcludes;
case SyntaxKind.MethodDeclaration:
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
return TransformFlags.MethodOrAccessorExcludes;
case SyntaxKind.AnyKeyword:
case SyntaxKind.NumberKeyword:
case SyntaxKind.NeverKeyword:
case SyntaxKind.StringKeyword:
case SyntaxKind.BooleanKeyword:
case SyntaxKind.SymbolKeyword:
case SyntaxKind.VoidKeyword:
case SyntaxKind.TypeParameter:
case SyntaxKind.PropertySignature:
case SyntaxKind.MethodSignature:
case SyntaxKind.CallSignature:
case SyntaxKind.ConstructSignature:
case SyntaxKind.IndexSignature:
case SyntaxKind.InterfaceDeclaration:
case SyntaxKind.TypeAliasDeclaration:
return TransformFlags.TypeExcludes;
case SyntaxKind.ObjectLiteralExpression:
return TransformFlags.ObjectLiteralExcludes;
default:
return TransformFlags.NodeExcludes;
}
}
}
60 changes: 0 additions & 60 deletions src/compiler/visitor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1270,66 +1270,6 @@ namespace ts {
return transformFlags | aggregateTransformFlagsForNode(child);
}

/**
* Gets the transform flags to exclude when unioning the transform flags of a subtree.
*
* NOTE: This needs to be kept up-to-date with the exclusions used in `computeTransformFlagsForNode`.
* For performance reasons, `computeTransformFlagsForNode` uses local constant values rather
* than calling this function.
*/
function getTransformFlagsSubtreeExclusions(kind: SyntaxKind) {
if (kind >= SyntaxKind.FirstTypeNode && kind <= SyntaxKind.LastTypeNode) {
return TransformFlags.TypeExcludes;
}

switch (kind) {
case SyntaxKind.CallExpression:
case SyntaxKind.NewExpression:
case SyntaxKind.ArrayLiteralExpression:
return TransformFlags.ArrayLiteralOrCallOrNewExcludes;
case SyntaxKind.ModuleDeclaration:
return TransformFlags.ModuleExcludes;
case SyntaxKind.Parameter:
return TransformFlags.ParameterExcludes;
case SyntaxKind.ArrowFunction:
return TransformFlags.ArrowFunctionExcludes;
case SyntaxKind.FunctionExpression:
case SyntaxKind.FunctionDeclaration:
return TransformFlags.FunctionExcludes;
case SyntaxKind.VariableDeclarationList:
return TransformFlags.VariableDeclarationListExcludes;
case SyntaxKind.ClassDeclaration:
case SyntaxKind.ClassExpression:
return TransformFlags.ClassExcludes;
case SyntaxKind.Constructor:
return TransformFlags.ConstructorExcludes;
case SyntaxKind.MethodDeclaration:
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
return TransformFlags.MethodOrAccessorExcludes;
case SyntaxKind.AnyKeyword:
case SyntaxKind.NumberKeyword:
case SyntaxKind.NeverKeyword:
case SyntaxKind.StringKeyword:
case SyntaxKind.BooleanKeyword:
case SyntaxKind.SymbolKeyword:
case SyntaxKind.VoidKeyword:
case SyntaxKind.TypeParameter:
case SyntaxKind.PropertySignature:
case SyntaxKind.MethodSignature:
case SyntaxKind.CallSignature:
case SyntaxKind.ConstructSignature:
case SyntaxKind.IndexSignature:
case SyntaxKind.InterfaceDeclaration:
case SyntaxKind.TypeAliasDeclaration:
return TransformFlags.TypeExcludes;
case SyntaxKind.ObjectLiteralExpression:
return TransformFlags.ObjectLiteralExcludes;
default:
return TransformFlags.NodeExcludes;
}
}

export namespace Debug {
export const failNotOptional = shouldAssert(AssertionLevel.Normal)
? (message?: string) => assert(false, message || "Node not optional.")
Expand Down
24 changes: 17 additions & 7 deletions src/harness/unittests/incrementalParser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ namespace ts {

// NOTE: 'reusedElements' is the expected count of elements reused from the old tree to the new
// tree. It may change as we tweak the parser. If the count increases then that should always
// be a good thing. If it decreases, that's not great (less reusability), but that may be
// unavoidable. If it does decrease an investigation should be done to make sure that things
// be a good thing. If it decreases, that's not great (less reusability), but that may be
// unavoidable. If it does decrease an investigation should be done to make sure that things
// are still ok and we're still appropriately reusing most of the tree.
function compareTrees(oldText: IScriptSnapshot, newText: IScriptSnapshot, textChangeRange: TextChangeRange, expectedReusedElements: number, oldTree?: SourceFile): SourceFile {
function compareTrees(oldText: IScriptSnapshot, newText: IScriptSnapshot, textChangeRange: TextChangeRange, expectedReusedElements: number, oldTree?: SourceFile) {
oldTree = oldTree || createTree(oldText, /*version:*/ ".");
Utils.assertInvariants(oldTree, /*parent:*/ undefined);

Expand Down Expand Up @@ -76,7 +76,7 @@ namespace ts {
assert.equal(actualReusedCount, expectedReusedElements, actualReusedCount + " !== " + expectedReusedElements);
}

return incrementalNewTree;
return { oldTree, newTree, incrementalNewTree };
}

function reusedElements(oldNode: SourceFile, newNode: SourceFile): number {
Expand All @@ -103,7 +103,7 @@ namespace ts {
for (let i = 0; i < repeat; i++) {
const oldText = ScriptSnapshot.fromString(source);
const newTextAndChange = withDelete(oldText, index, 1);
const newTree = compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, -1, oldTree);
const newTree = compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, -1, oldTree).incrementalNewTree;

source = newTextAndChange.text.getText(0, newTextAndChange.text.getLength());
oldTree = newTree;
Expand All @@ -116,7 +116,7 @@ namespace ts {
for (let i = 0; i < repeat; i++) {
const oldText = ScriptSnapshot.fromString(source);
const newTextAndChange = withInsert(oldText, index + i, toInsert.charAt(i));
const newTree = compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, -1, oldTree);
const newTree = compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, -1, oldTree).incrementalNewTree;

source = newTextAndChange.text.getText(0, newTextAndChange.text.getLength());
oldTree = newTree;
Expand Down Expand Up @@ -639,7 +639,7 @@ module m3 { }\
});

it("Unterminated comment after keyword converted to identifier", () => {
// 'public' as a keyword should be incrementally unusable (because it has an
// 'public' as a keyword should be incrementally unusable (because it has an
// unterminated comment). When we convert it to an identifier, that shouldn't
// change anything, and we should still get the same errors.
const source = "return; a.public /*";
Expand Down Expand Up @@ -796,6 +796,16 @@ module m3 { }\
compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 4);
});

it("Reuse transformFlags of subtree during bind", () => {
const source = `class Greeter { constructor(element: HTMLElement) { } }`;
const oldText = ScriptSnapshot.fromString(source);
const newTextAndChange = withChange(oldText, 15, 0, "\n");
const { oldTree, incrementalNewTree } = compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, -1);
bindSourceFile(oldTree, {});
bindSourceFile(incrementalNewTree, {});
assert.equal(oldTree.transformFlags, incrementalNewTree.transformFlags);
});

// Simulated typing tests.

it("Type extends clause 1", () => {
Expand Down
2 changes: 0 additions & 2 deletions src/services/services.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,13 @@ namespace ts {
public jsDocComments: JSDoc[];
public original: Node;
public transformFlags: TransformFlags;
public excludeTransformFlags: TransformFlags;
private _children: Node[];

constructor(kind: SyntaxKind, pos: number, end: number) {
this.pos = pos;
this.end = end;
this.flags = NodeFlags.None;
this.transformFlags = undefined;
this.excludeTransformFlags = undefined;
this.parent = undefined;
this.kind = kind;
}
Expand Down

0 comments on commit ddc4ae7

Please sign in to comment.