Skip to content

Commit 68b0323

Browse files
authored
Refactor emit substitution into transform (microsoft#42676)
* 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
1 parent df5ffc0 commit 68b0323

29 files changed

+5683
-3940
lines changed

Diff for: scripts/perf-result-post.js

+78-37
Original file line numberDiff line numberDiff line change
@@ -3,45 +3,86 @@
33
// Must reference esnext.asynciterable lib, since octokit uses AsyncIterable internally
44
const { Octokit } = require("@octokit/rest");
55
const fs = require("fs");
6+
const ado = require("azure-devops-node-api");
7+
const { default: fetch } = require("node-fetch");
68

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

15-
const gh = new Octokit({
16-
auth: process.argv[2]
17-
});
18-
gh.issues.createComment({
19-
issue_number: +source,
20-
owner: "Microsoft",
21-
repo: "TypeScript",
22-
body: `@${requester}
23-
The results of the perf run you requested are in!
24-
<details><summary> Here they are:</summary><p>
25-
${outputTableText}
26-
</p></details>`
27-
}).then(async data => {
28-
console.log(`Results posted!`);
29-
const newCommentUrl = data.data.html_url;
30-
const comment = await gh.issues.getComment({
31-
owner: "Microsoft",
32-
repo: "TypeScript",
33-
comment_id: +postedComment
34-
});
35-
const newBody = `${comment.data.body}
36-
37-
Update: [The results are in!](${newCommentUrl})`;
38-
return await gh.issues.updateComment({
39-
owner: "Microsoft",
40-
repo: "TypeScript",
41-
comment_id: +postedComment,
42-
body: newBody
43-
});
44-
}).catch(e => {
10+
async function main() {
11+
const source = process.env.SOURCE_ISSUE;
12+
if (!source) throw new Error("SOURCE_ISSUE environment variable not set.");
13+
14+
const requester = process.env.REQUESTING_USER;
15+
if (!requester) throw new Error("REQUESTING_USER environment variable not set.");
16+
17+
const buildId = process.env.BUILD_BUILDID;
18+
if (!requester) throw new Error("BUILD_BUILDID environment variable not set.");
19+
20+
const postedComment = process.env.STATUS_COMMENT;
21+
if (!postedComment) throw new Error("STATUS_COMMENT environment variable not set.");
22+
23+
const [auth, fragment, includeArtifact] = process.argv.slice(2);
24+
if (!auth) throw new Error("First argument must be a GitHub auth token.");
25+
if (!fragment) throw new Error("Second argument must be a path to an HTML fragment.");
26+
27+
const gh = new Octokit({ auth });
28+
try {
29+
console.log(`Loading fragment from ${fragment}...`);
30+
const outputTableText = fs.readFileSync(fragment, { encoding: "utf8" });
31+
console.log(`Fragment contents:\n${outputTableText}`);
32+
33+
let benchmarkText = "";
34+
if (includeArtifact === "--include-artifact") {
35+
// post a link to the benchmark file
36+
const cli = new ado.WebApi("https://typescript.visualstudio.com/defaultcollection", ado.getHandlerFromToken("")); // Empty token, anon auth
37+
const build = await cli.getBuildApi();
38+
const artifact = await build.getArtifact("typescript", +buildId, "benchmark");
39+
const updatedUrl = new URL(artifact.resource.url);
40+
updatedUrl.search = `artifactName=benchmark&fileId=${artifact.resource.data}&fileName=manifest`;
41+
const resp = await (await fetch(`${updatedUrl}`)).json();
42+
for (const file of resp.items) {
43+
if (/[\\/]linux\.benchmark$/.test(file.path)) {
44+
const benchmarkUrl = new URL(artifact.resource.url);
45+
benchmarkUrl.search = `artifactName=benchmark&fileId=${file.blob.id}&fileName=linux.benchmark`;
46+
benchmarkText = `\n<details><summary>Developer Information:</summary><p><a href="${benchmarkUrl.href}">Download Benchmark</a></p></details>\n`;
47+
break;
48+
}
49+
}
50+
}
51+
52+
const data = await gh.issues.createComment({
53+
issue_number: +source,
54+
owner: "Microsoft",
55+
repo: "TypeScript",
56+
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>`
57+
});
58+
59+
console.log(`Results posted!`);
60+
const newCommentUrl = data.data.html_url;
61+
const comment = await gh.issues.getComment({
62+
owner: "Microsoft",
63+
repo: "TypeScript",
64+
comment_id: +postedComment
65+
});
66+
const newBody = `${comment.data.body}\n\nUpdate: [The results are in!](${newCommentUrl})`;
67+
await gh.issues.updateComment({
68+
owner: "Microsoft",
69+
repo: "TypeScript",
70+
comment_id: +postedComment,
71+
body: newBody
72+
});
73+
}
74+
catch (e) {
75+
const gh = new Octokit({ auth });
76+
await gh.issues.createComment({
77+
issue_number: +source,
78+
owner: "Microsoft",
79+
repo: "TypeScript",
80+
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)).`
81+
});
82+
}
83+
}
84+
85+
main().catch(e => {
4586
console.error(e);
4687
process.exit(1);
4788
});

Diff for: src/compiler/binder.ts

+90-111
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ namespace ts {
227227

228228
const unreachableFlow: FlowNode = { flags: FlowFlags.Unreachable };
229229
const reportedUnreachableFlow: FlowNode = { flags: FlowFlags.Unreachable };
230+
const bindBinaryExpressionFlow = createBindBinaryExpressionFlow();
230231

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

1500-
const enum BindBinaryExpressionFlowState {
1501-
BindThenBindChildren,
1502-
MaybeBindLeft,
1503-
BindToken,
1504-
BindRight,
1505-
FinishBind
1506-
}
1507-
1508-
function bindBinaryExpressionFlow(node: BinaryExpression) {
1509-
const workStacks: {
1510-
expr: BinaryExpression[],
1511-
state: BindBinaryExpressionFlowState[],
1512-
inStrictMode: (boolean | undefined)[],
1513-
parent: (Node | undefined)[],
1514-
} = {
1515-
expr: [node],
1516-
state: [BindBinaryExpressionFlowState.MaybeBindLeft],
1517-
inStrictMode: [undefined],
1518-
parent: [undefined],
1519-
};
1520-
let stackIndex = 0;
1521-
while (stackIndex >= 0) {
1522-
node = workStacks.expr[stackIndex];
1523-
switch (workStacks.state[stackIndex]) {
1524-
case BindBinaryExpressionFlowState.BindThenBindChildren: {
1525-
// This state is used only when recuring, to emulate the work that `bind` does before
1526-
// reaching `bindChildren`. A normal call to `bindBinaryExpressionFlow` will already have done this work.
1527-
setParent(node, parent);
1528-
const saveInStrictMode = inStrictMode;
1529-
bindWorker(node);
1530-
const saveParent = parent;
1531-
parent = node;
1532-
1533-
advanceState(BindBinaryExpressionFlowState.MaybeBindLeft, saveInStrictMode, saveParent);
1534-
break;
1535-
}
1536-
case BindBinaryExpressionFlowState.MaybeBindLeft: {
1537-
const operator = node.operatorToken.kind;
1538-
// TODO: bindLogicalExpression is recursive - if we want to handle deeply nested `&&` expressions
1539-
// we'll need to handle the `bindLogicalExpression` scenarios in this state machine, too
1540-
// For now, though, since the common cases are chained `+`, leaving it recursive is fine
1541-
if (operator === SyntaxKind.AmpersandAmpersandToken || operator === SyntaxKind.BarBarToken || operator === SyntaxKind.QuestionQuestionToken ||
1542-
isLogicalOrCoalescingAssignmentOperator(operator)) {
1543-
if (isTopLevelLogicalExpression(node)) {
1544-
const postExpressionLabel = createBranchLabel();
1545-
bindLogicalLikeExpression(node, postExpressionLabel, postExpressionLabel);
1546-
currentFlow = finishFlowLabel(postExpressionLabel);
1547-
}
1548-
else {
1549-
bindLogicalLikeExpression(node, currentTrueTarget!, currentFalseTarget!);
1550-
}
1551-
completeNode();
1552-
}
1553-
else {
1554-
advanceState(BindBinaryExpressionFlowState.BindToken);
1555-
maybeBind(node.left);
1556-
}
1557-
break;
1558-
}
1559-
case BindBinaryExpressionFlowState.BindToken: {
1560-
if (node.operatorToken.kind === SyntaxKind.CommaToken) {
1561-
maybeBindExpressionFlowIfCall(node.left);
1562-
}
1563-
advanceState(BindBinaryExpressionFlowState.BindRight);
1564-
maybeBind(node.operatorToken);
1565-
break;
1566-
}
1567-
case BindBinaryExpressionFlowState.BindRight: {
1568-
advanceState(BindBinaryExpressionFlowState.FinishBind);
1569-
maybeBind(node.right);
1570-
break;
1501+
function createBindBinaryExpressionFlow() {
1502+
interface WorkArea {
1503+
stackIndex: number;
1504+
skip: boolean;
1505+
inStrictModeStack: (boolean | undefined)[];
1506+
parentStack: (Node | undefined)[];
1507+
}
1508+
1509+
return createBinaryExpressionTrampoline(onEnter, onLeft, onOperator, onRight, onExit, /*foldState*/ undefined);
1510+
1511+
function onEnter(node: BinaryExpression, state: WorkArea | undefined) {
1512+
if (state) {
1513+
state.stackIndex++;
1514+
// Emulate the work that `bind` does before reaching `bindChildren`. A normal call to
1515+
// `bindBinaryExpressionFlow` will already have done this work.
1516+
setParent(node, parent);
1517+
const saveInStrictMode = inStrictMode;
1518+
bindWorker(node);
1519+
const saveParent = parent;
1520+
parent = node;
1521+
state.skip = false;
1522+
state.inStrictModeStack[state.stackIndex] = saveInStrictMode;
1523+
state.parentStack[state.stackIndex] = saveParent;
1524+
}
1525+
else {
1526+
state = {
1527+
stackIndex: 0,
1528+
skip: false,
1529+
inStrictModeStack: [undefined],
1530+
parentStack: [undefined]
1531+
};
1532+
}
1533+
// TODO: bindLogicalExpression is recursive - if we want to handle deeply nested `&&` expressions
1534+
// we'll need to handle the `bindLogicalExpression` scenarios in this state machine, too
1535+
// For now, though, since the common cases are chained `+`, leaving it recursive is fine
1536+
const operator = node.operatorToken.kind;
1537+
if (operator === SyntaxKind.AmpersandAmpersandToken ||
1538+
operator === SyntaxKind.BarBarToken ||
1539+
operator === SyntaxKind.QuestionQuestionToken ||
1540+
isLogicalOrCoalescingAssignmentOperator(operator)) {
1541+
if (isTopLevelLogicalExpression(node)) {
1542+
const postExpressionLabel = createBranchLabel();
1543+
bindLogicalLikeExpression(node, postExpressionLabel, postExpressionLabel);
1544+
currentFlow = finishFlowLabel(postExpressionLabel);
15711545
}
1572-
case BindBinaryExpressionFlowState.FinishBind: {
1573-
const operator = node.operatorToken.kind;
1574-
if (isAssignmentOperator(operator) && !isAssignmentTarget(node)) {
1575-
bindAssignmentTargetFlow(node.left);
1576-
if (operator === SyntaxKind.EqualsToken && node.left.kind === SyntaxKind.ElementAccessExpression) {
1577-
const elementAccess = <ElementAccessExpression>node.left;
1578-
if (isNarrowableOperand(elementAccess.expression)) {
1579-
currentFlow = createFlowMutation(FlowFlags.ArrayMutation, currentFlow, node);
1580-
}
1581-
}
1582-
}
1583-
completeNode();
1584-
break;
1546+
else {
1547+
bindLogicalLikeExpression(node, currentTrueTarget!, currentFalseTarget!);
15851548
}
1586-
default: return Debug.fail(`Invalid state ${workStacks.state[stackIndex]} for bindBinaryExpressionFlow`);
1549+
state.skip = true;
15871550
}
1551+
return state;
15881552
}
15891553

1590-
/**
1591-
* Note that `advanceState` sets the _current_ head state, and that `maybeBind` potentially pushes on a new
1592-
* head state; so `advanceState` must be called before any `maybeBind` during a state's execution.
1593-
*/
1594-
function advanceState(state: BindBinaryExpressionFlowState, isInStrictMode?: boolean, parent?: Node) {
1595-
workStacks.state[stackIndex] = state;
1596-
if (isInStrictMode !== undefined) {
1597-
workStacks.inStrictMode[stackIndex] = isInStrictMode;
1554+
function onLeft(left: Expression, state: WorkArea, _node: BinaryExpression) {
1555+
if (!state.skip) {
1556+
return maybeBind(left);
15981557
}
1599-
if (parent !== undefined) {
1600-
workStacks.parent[stackIndex] = parent;
1558+
}
1559+
1560+
function onOperator(operatorToken: BinaryOperatorToken, state: WorkArea, node: BinaryExpression) {
1561+
if (!state.skip) {
1562+
if (operatorToken.kind === SyntaxKind.CommaToken) {
1563+
maybeBindExpressionFlowIfCall(node.left);
1564+
}
1565+
bind(operatorToken);
16011566
}
16021567
}
16031568

1604-
function completeNode() {
1605-
if (workStacks.inStrictMode[stackIndex] !== undefined) {
1606-
inStrictMode = workStacks.inStrictMode[stackIndex]!;
1607-
parent = workStacks.parent[stackIndex]!;
1569+
function onRight(right: Expression, state: WorkArea, _node: BinaryExpression) {
1570+
if (!state.skip) {
1571+
return maybeBind(right);
16081572
}
1609-
stackIndex--;
16101573
}
16111574

1612-
/**
1613-
* If `node` is a BinaryExpression, adds it to the local work stack, otherwise recursively binds it
1614-
*/
1575+
function onExit(node: BinaryExpression, state: WorkArea) {
1576+
if (!state.skip) {
1577+
const operator = node.operatorToken.kind;
1578+
if (isAssignmentOperator(operator) && !isAssignmentTarget(node)) {
1579+
bindAssignmentTargetFlow(node.left);
1580+
if (operator === SyntaxKind.EqualsToken && node.left.kind === SyntaxKind.ElementAccessExpression) {
1581+
const elementAccess = <ElementAccessExpression>node.left;
1582+
if (isNarrowableOperand(elementAccess.expression)) {
1583+
currentFlow = createFlowMutation(FlowFlags.ArrayMutation, currentFlow, node);
1584+
}
1585+
}
1586+
}
1587+
}
1588+
const savedInStrictMode = state.inStrictModeStack[state.stackIndex];
1589+
const savedParent = state.parentStack[state.stackIndex];
1590+
if (savedInStrictMode !== undefined) {
1591+
inStrictMode = savedInStrictMode;
1592+
}
1593+
if (savedParent !== undefined) {
1594+
parent = savedParent;
1595+
}
1596+
state.skip = false;
1597+
state.stackIndex--;
1598+
}
1599+
16151600
function maybeBind(node: Node) {
16161601
if (node && isBinaryExpression(node) && !isDestructuringAssignment(node)) {
1617-
stackIndex++;
1618-
workStacks.expr[stackIndex] = node;
1619-
workStacks.state[stackIndex] = BindBinaryExpressionFlowState.BindThenBindChildren;
1620-
workStacks.inStrictMode[stackIndex] = undefined;
1621-
workStacks.parent[stackIndex] = undefined;
1622-
}
1623-
else {
1624-
bind(node);
1602+
return node;
16251603
}
1604+
bind(node);
16261605
}
16271606
}
16281607

Diff for: src/compiler/debug.ts

+8
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,14 @@ namespace ts {
275275
}
276276
}
277277

278+
/**
279+
* Asserts a value has the specified type in typespace only (does not perform a runtime assertion).
280+
* This is useful in cases where we switch on `node.kind` and can be reasonably sure the type is accurate, and
281+
* as a result can reduce the number of unnecessary casts.
282+
*/
283+
export function type<T>(value: unknown): asserts value is T;
284+
export function type(_value: unknown) { }
285+
278286
export function getFunctionName(func: AnyFunction) {
279287
if (typeof func !== "function") {
280288
return "";

0 commit comments

Comments
 (0)