Skip to content

Commit

Permalink
Refactor emit substitution into transform (#42676)
Browse files Browse the repository at this point in the history 
* Refactor emit substitution into transform

* Add reusable state machine for binary expressions

* Allow emitBinary to use state machine for comments/sourcemaps

* Switch core trampoline state back to arrays

* Switch binder to parallel stacks, temporarily partially revert emitBinary

* Add link to benchmark when posting perf results

* Ensure work stacks are per-execution

* Reenable comments and sourcemaps
  • Loading branch information
@rbuckton
rbuckton authored Feb 26, 2021
1 parent df5ffc0 commit 68b0323
Show file tree
Hide file tree
Showing 29 changed files with 5,683 additions and 3,940 deletions.
115 changes: 78 additions & 37 deletions scripts/perf-result-post.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,45 +3,86 @@
// Must reference esnext.asynciterable lib, since octokit uses AsyncIterable internally
const { Octokit } = require("@octokit/rest");
const fs = require("fs");
const ado = require("azure-devops-node-api");
const { default: fetch } = require("node-fetch");

const requester = process.env.requesting_user;
const source = process.env.source_issue;
const postedComment = process.env.status_comment;
console.log(`Loading fragment from ${process.argv[3]}...`);
const outputTableText = fs.readFileSync(process.argv[3], { encoding: "utf8" });
console.log(`Fragment contents:
${outputTableText}`);

const gh = new Octokit({
auth: process.argv[2]
});
gh.issues.createComment({
issue_number: +source,
owner: "Microsoft",
repo: "TypeScript",
body: `@${requester}
The results of the perf run you requested are in!
<details><summary> Here they are:</summary><p>
${outputTableText}
</p></details>`
}).then(async data => {
console.log(`Results posted!`);
const newCommentUrl = data.data.html_url;
const comment = await gh.issues.getComment({
owner: "Microsoft",
repo: "TypeScript",
comment_id: +postedComment
});
const newBody = `${comment.data.body}
Update: [The results are in!](${newCommentUrl})`;
return await gh.issues.updateComment({
owner: "Microsoft",
repo: "TypeScript",
comment_id: +postedComment,
body: newBody
});
}).catch(e => {
async function main() {
const source = process.env.SOURCE_ISSUE;
if (!source) throw new Error("SOURCE_ISSUE environment variable not set.");

const requester = process.env.REQUESTING_USER;
if (!requester) throw new Error("REQUESTING_USER environment variable not set.");

const buildId = process.env.BUILD_BUILDID;
if (!requester) throw new Error("BUILD_BUILDID environment variable not set.");

const postedComment = process.env.STATUS_COMMENT;
if (!postedComment) throw new Error("STATUS_COMMENT environment variable not set.");

const [auth, fragment, includeArtifact] = process.argv.slice(2);
if (!auth) throw new Error("First argument must be a GitHub auth token.");
if (!fragment) throw new Error("Second argument must be a path to an HTML fragment.");

const gh = new Octokit({ auth });
try {
console.log(`Loading fragment from ${fragment}...`);
const outputTableText = fs.readFileSync(fragment, { encoding: "utf8" });
console.log(`Fragment contents:\n${outputTableText}`);

let benchmarkText = "";
if (includeArtifact === "--include-artifact") {
// post a link to the benchmark file
const cli = new ado.WebApi("https://typescript.visualstudio.com/defaultcollection", ado.getHandlerFromToken("")); // Empty token, anon auth
const build = await cli.getBuildApi();
const artifact = await build.getArtifact("typescript", +buildId, "benchmark");
const updatedUrl = new URL(artifact.resource.url);
updatedUrl.search = `artifactName=benchmark&fileId=${artifact.resource.data}&fileName=manifest`;
const resp = await (await fetch(`${updatedUrl}`)).json();
for (const file of resp.items) {
if (/[\\/]linux\.benchmark$/.test(file.path)) {
const benchmarkUrl = new URL(artifact.resource.url);
benchmarkUrl.search = `artifactName=benchmark&fileId=${file.blob.id}&fileName=linux.benchmark`;
benchmarkText = `\n<details><summary>Developer Information:</summary><p><a href="${benchmarkUrl.href}">Download Benchmark</a></p></details>\n`;
break;
}
}
}

const data = await gh.issues.createComment({
issue_number: +source,
owner: "Microsoft",
repo: "TypeScript",
body: `@${requester}\nThe results of the perf run you requested are in!\n<details><summary> Here they are:</summary><p>\n${outputTableText}\n</p>${benchmarkText}</details>`
});

console.log(`Results posted!`);
const newCommentUrl = data.data.html_url;
const comment = await gh.issues.getComment({
owner: "Microsoft",
repo: "TypeScript",
comment_id: +postedComment
});
const newBody = `${comment.data.body}\n\nUpdate: [The results are in!](${newCommentUrl})`;
await gh.issues.updateComment({
owner: "Microsoft",
repo: "TypeScript",
comment_id: +postedComment,
body: newBody
});
}
catch (e) {
const gh = new Octokit({ auth });
await gh.issues.createComment({
issue_number: +source,
owner: "Microsoft",
repo: "TypeScript",
body: `Hey @${requester}, something went wrong when publishing results. ([You can check the log here](https://typescript.visualstudio.com/TypeScript/_build/index?buildId=${buildId}&_a=summary)).`
});
}
}

main().catch(e => {
console.error(e);
process.exit(1);
});
201 changes: 90 additions & 111 deletions src/compiler/binder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ namespace ts {

const unreachableFlow: FlowNode = { flags: FlowFlags.Unreachable };
const reportedUnreachableFlow: FlowNode = { flags: FlowFlags.Unreachable };
const bindBinaryExpressionFlow = createBindBinaryExpressionFlow();

/**
* Inside the binder, we may create a diagnostic for an as-yet unbound node (with potentially no parent pointers, implying no accessible source file)
Expand Down Expand Up @@ -1497,132 +1498,110 @@ namespace ts {
bindAssignmentTargetFlow(node.left);
}

const enum BindBinaryExpressionFlowState {
BindThenBindChildren,
MaybeBindLeft,
BindToken,
BindRight,
FinishBind
}

function bindBinaryExpressionFlow(node: BinaryExpression) {
const workStacks: {
expr: BinaryExpression[],
state: BindBinaryExpressionFlowState[],
inStrictMode: (boolean | undefined)[],
parent: (Node | undefined)[],
} = {
expr: [node],
state: [BindBinaryExpressionFlowState.MaybeBindLeft],
inStrictMode: [undefined],
parent: [undefined],
};
let stackIndex = 0;
while (stackIndex >= 0) {
node = workStacks.expr[stackIndex];
switch (workStacks.state[stackIndex]) {
case BindBinaryExpressionFlowState.BindThenBindChildren: {
// This state is used only when recuring, to emulate the work that `bind` does before
// reaching `bindChildren`. A normal call to `bindBinaryExpressionFlow` will already have done this work.
setParent(node, parent);
const saveInStrictMode = inStrictMode;
bindWorker(node);
const saveParent = parent;
parent = node;

advanceState(BindBinaryExpressionFlowState.MaybeBindLeft, saveInStrictMode, saveParent);
break;
}
case BindBinaryExpressionFlowState.MaybeBindLeft: {
const operator = node.operatorToken.kind;
// TODO: bindLogicalExpression is recursive - if we want to handle deeply nested `&&` expressions
// we'll need to handle the `bindLogicalExpression` scenarios in this state machine, too
// For now, though, since the common cases are chained `+`, leaving it recursive is fine
if (operator === SyntaxKind.AmpersandAmpersandToken || operator === SyntaxKind.BarBarToken || operator === SyntaxKind.QuestionQuestionToken ||
isLogicalOrCoalescingAssignmentOperator(operator)) {
if (isTopLevelLogicalExpression(node)) {
const postExpressionLabel = createBranchLabel();
bindLogicalLikeExpression(node, postExpressionLabel, postExpressionLabel);
currentFlow = finishFlowLabel(postExpressionLabel);
}
else {
bindLogicalLikeExpression(node, currentTrueTarget!, currentFalseTarget!);
}
completeNode();
}
else {
advanceState(BindBinaryExpressionFlowState.BindToken);
maybeBind(node.left);
}
break;
}
case BindBinaryExpressionFlowState.BindToken: {
if (node.operatorToken.kind === SyntaxKind.CommaToken) {
maybeBindExpressionFlowIfCall(node.left);
}
advanceState(BindBinaryExpressionFlowState.BindRight);
maybeBind(node.operatorToken);
break;
}
case BindBinaryExpressionFlowState.BindRight: {
advanceState(BindBinaryExpressionFlowState.FinishBind);
maybeBind(node.right);
break;
function createBindBinaryExpressionFlow() {
interface WorkArea {
stackIndex: number;
skip: boolean;
inStrictModeStack: (boolean | undefined)[];
parentStack: (Node | undefined)[];
}

return createBinaryExpressionTrampoline(onEnter, onLeft, onOperator, onRight, onExit, /*foldState*/ undefined);

function onEnter(node: BinaryExpression, state: WorkArea | undefined) {
if (state) {
state.stackIndex++;
// Emulate the work that `bind` does before reaching `bindChildren`. A normal call to
// `bindBinaryExpressionFlow` will already have done this work.
setParent(node, parent);
const saveInStrictMode = inStrictMode;
bindWorker(node);
const saveParent = parent;
parent = node;
state.skip = false;
state.inStrictModeStack[state.stackIndex] = saveInStrictMode;
state.parentStack[state.stackIndex] = saveParent;
}
else {
state = {
stackIndex: 0,
skip: false,
inStrictModeStack: [undefined],
parentStack: [undefined]
};
}
// TODO: bindLogicalExpression is recursive - if we want to handle deeply nested `&&` expressions
// we'll need to handle the `bindLogicalExpression` scenarios in this state machine, too
// For now, though, since the common cases are chained `+`, leaving it recursive is fine
const operator = node.operatorToken.kind;
if (operator === SyntaxKind.AmpersandAmpersandToken ||
operator === SyntaxKind.BarBarToken ||
operator === SyntaxKind.QuestionQuestionToken ||
isLogicalOrCoalescingAssignmentOperator(operator)) {
if (isTopLevelLogicalExpression(node)) {
const postExpressionLabel = createBranchLabel();
bindLogicalLikeExpression(node, postExpressionLabel, postExpressionLabel);
currentFlow = finishFlowLabel(postExpressionLabel);
}
case BindBinaryExpressionFlowState.FinishBind: {
const operator = node.operatorToken.kind;
if (isAssignmentOperator(operator) && !isAssignmentTarget(node)) {
bindAssignmentTargetFlow(node.left);
if (operator === SyntaxKind.EqualsToken && node.left.kind === SyntaxKind.ElementAccessExpression) {
const elementAccess = <ElementAccessExpression>node.left;
if (isNarrowableOperand(elementAccess.expression)) {
currentFlow = createFlowMutation(FlowFlags.ArrayMutation, currentFlow, node);
}
}
}
completeNode();
break;
else {
bindLogicalLikeExpression(node, currentTrueTarget!, currentFalseTarget!);
}
default: return Debug.fail(`Invalid state ${workStacks.state[stackIndex]} for bindBinaryExpressionFlow`);
state.skip = true;
}
return state;
}

/**
* Note that `advanceState` sets the _current_ head state, and that `maybeBind` potentially pushes on a new
* head state; so `advanceState` must be called before any `maybeBind` during a state's execution.
*/
function advanceState(state: BindBinaryExpressionFlowState, isInStrictMode?: boolean, parent?: Node) {
workStacks.state[stackIndex] = state;
if (isInStrictMode !== undefined) {
workStacks.inStrictMode[stackIndex] = isInStrictMode;
function onLeft(left: Expression, state: WorkArea, _node: BinaryExpression) {
if (!state.skip) {
return maybeBind(left);
}
if (parent !== undefined) {
workStacks.parent[stackIndex] = parent;
}

function onOperator(operatorToken: BinaryOperatorToken, state: WorkArea, node: BinaryExpression) {
if (!state.skip) {
if (operatorToken.kind === SyntaxKind.CommaToken) {
maybeBindExpressionFlowIfCall(node.left);
}
bind(operatorToken);
}
}

function completeNode() {
if (workStacks.inStrictMode[stackIndex] !== undefined) {
inStrictMode = workStacks.inStrictMode[stackIndex]!;
parent = workStacks.parent[stackIndex]!;
function onRight(right: Expression, state: WorkArea, _node: BinaryExpression) {
if (!state.skip) {
return maybeBind(right);
}
stackIndex--;
}

/**
* If `node` is a BinaryExpression, adds it to the local work stack, otherwise recursively binds it
*/
function onExit(node: BinaryExpression, state: WorkArea) {
if (!state.skip) {
const operator = node.operatorToken.kind;
if (isAssignmentOperator(operator) && !isAssignmentTarget(node)) {
bindAssignmentTargetFlow(node.left);
if (operator === SyntaxKind.EqualsToken && node.left.kind === SyntaxKind.ElementAccessExpression) {
const elementAccess = <ElementAccessExpression>node.left;
if (isNarrowableOperand(elementAccess.expression)) {
currentFlow = createFlowMutation(FlowFlags.ArrayMutation, currentFlow, node);
}
}
}
}
const savedInStrictMode = state.inStrictModeStack[state.stackIndex];
const savedParent = state.parentStack[state.stackIndex];
if (savedInStrictMode !== undefined) {
inStrictMode = savedInStrictMode;
}
if (savedParent !== undefined) {
parent = savedParent;
}
state.skip = false;
state.stackIndex--;
}

function maybeBind(node: Node) {
if (node && isBinaryExpression(node) && !isDestructuringAssignment(node)) {
stackIndex++;
workStacks.expr[stackIndex] = node;
workStacks.state[stackIndex] = BindBinaryExpressionFlowState.BindThenBindChildren;
workStacks.inStrictMode[stackIndex] = undefined;
workStacks.parent[stackIndex] = undefined;
}
else {
bind(node);
return node;
}
bind(node);
}
}

Expand Down
8 changes: 8 additions & 0 deletions src/compiler/debug.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,14 @@ namespace ts {
}
}

/**
* Asserts a value has the specified type in typespace only (does not perform a runtime assertion).
* This is useful in cases where we switch on `node.kind` and can be reasonably sure the type is accurate, and
* as a result can reduce the number of unnecessary casts.
*/
export function type<T>(value: unknown): asserts value is T;
export function type(_value: unknown) { }

export function getFunctionName(func: AnyFunction) {
if (typeof func !== "function") {
return "";
Expand Down
Loading

0 comments on commit 68b0323

Please sign in to comment.