@samitouri / QOS-React-2 / commits / d6505572cb

[valueblocks] Disallow AssignmentExpression in value blocks (temporarily)

There's a bug with assignment expression in normal value blocks due to LeaveSSA. Until that's resolved i'm temporarily distinguishing "loop" blocks and "value" blocks, and disallowing assignment expressions in value blocks specifically.

Joe Savona committed Jan 31, 2023 at 13:39 UTC d6505572cb1de2542303b98d7280c7bef8ca1145
7 files changed +28 -11
compiler/forget/src/HIR/BuildHIR.ts
+15 -6
@@ -247,11 +247,11 @@ function lowerStatement(
247 case "ForStatement": {
248 const stmt = stmtPath as NodePath<t.ForStatement>;
249
250 - const testBlock = builder.reserve("value");
250 + const testBlock = builder.reserve("loop");
251 // Block for code following the loop
252 const continuationBlock = builder.reserve("block");
253
254 - const initBlock = builder.enter("value", (blockId) => {
254 + const initBlock = builder.enter("loop", (blockId) => {
255 const init = stmt.get("init");
256 if (!init.isVariableDeclaration()) {
257 builder.errors.push({
@@ -271,7 +271,7 @@ function lowerStatement(
271 };
272 });
273
274 - const updateBlock = builder.enter("value", (blockId) => {
274 + const updateBlock = builder.enter("loop", (blockId) => {
275 const update = stmt.get("update");
276 if (update.node == null) {
277 builder.errors.push({
@@ -342,7 +342,7 @@ function lowerStatement(
342 case "WhileStatement": {
343 const stmt = stmtPath as NodePath<t.WhileStatement>;
344 // Block used to evaluate whether to (re)enter or exit the loop
345 - const conditionalBlock = builder.reserve("value");
345 + const conditionalBlock = builder.reserve("loop");
346 // Block for code following the loop
347 const continuationBlock = builder.reserve("block");
348 // Loop body
@@ -899,7 +899,7 @@ function lowerExpression(
899 const place = buildTemporaryPlace(builder, exprLoc);
900
901 // Block for the consequent (if the test is truthy)
902 - const consequentBlock = builder.enter("block", (blockId) => {
902 + const consequentBlock = builder.enter("value", (blockId) => {
903 builder.push({
904 id: makeInstructionId(0),
905 lvalue: { kind: InstructionKind.Reassign, place: { ...place } },
@@ -914,7 +914,7 @@ function lowerExpression(
914 };
915 });
916 // Block for the alternate (if the test is not truthy)
917 - const alternateBlock = builder.enter("block", (blockId) => {
917 + const alternateBlock = builder.enter("value", (blockId) => {
918 builder.push({
919 id: makeInstructionId(0),
920 lvalue: { kind: InstructionKind.Reassign, place: { ...place } },
@@ -1023,6 +1023,15 @@ function lowerExpression(
1023 const expr = exprPath as NodePath<t.AssignmentExpression>;
1024 const operator = expr.node.operator;
1025
1026 + if (builder.currentBlockKind() === "value") {
1027 + builder.errors.push({
1028 + reason: `(BuildHIR::lowerExpression) Handle AssignmentExpression within a LogicalExpression or ConditionalExpression`,
1029 + severity: ErrorSeverity.Todo,
1030 + nodePath: expr,
1031 + });
1032 + return { kind: "UnsupportedNode", node: exprNode, loc: exprLoc };
1033 + }
1034 +
1035 if (operator === "=") {
1036 const left = expr.get("left");
1037 return lowerAssignment(
compiler/forget/src/HIR/HIR.ts
+1 -1
@@ -223,7 +223,7 @@ export type HIR = {
223 * an exception occurs, therefore the block model only represents explicit throw
224 * statements and not implicit exceptions which may occur.
225 */
226 -export type BlockKind = "block" | "value";
226 +export type BlockKind = "block" | "value" | "loop";
227 export type BasicBlock = {
228 kind: BlockKind;
229 id: BlockId;
compiler/forget/src/HIR/MergeConsecutiveBlocks.ts
+2 -2
@@ -21,7 +21,7 @@ import {
21 * (ie ends in a goto) and where the predecessor is the only predecessor
22 * for that successor (ie, there is no other way to reach the successor).
23 *
24 - * Note that this pass leaves "value" blocks alone because they cannot
24 + * Note that this pass leaves value/loop blocks alone because they cannot
25 * be merged without breaking the structure of the high-level terminals
26 * that reference them.
27 *
@@ -32,7 +32,7 @@ export function mergeConsecutiveBlocks(fn: HIRFunction): void {
32 for (const [, block] of fn.body.blocks) {
33 // Can only merge blocks with a single predecessor, can't merge
34 // value blocks
35 - if (block.kind === "value" || block.preds.size !== 1) {
35 + if (block.kind !== "block" || block.preds.size !== 1) {
36 continue;
37 }
38 const originalPredecessorId = Array.from(block.preds)[0]!;
compiler/forget/src/Optimization/ConstantPropagation.ts
+1 -1
@@ -104,7 +104,7 @@ function applyConstantPropagation(fn: HIRFunction): boolean {
104 }
105 }
106
107 - if (block.kind === "value") {
107 + if (block.kind !== "block") {
108 // can't rewrite terminals in value blocks yet
109 continue;
110 }
compiler/forget/src/__tests__/fixtures/hir/ternary-assignment-expression.js
+1 -1
@@ -1,4 +1,4 @@
1 -// @only
1 +// @skip
2 function ternary(props) {
3 let x = 0;
4 const y = props.a ? (x = 1) : (x = 2);
compiler/forget/src/__tests__/fixtures/hir/while-logical.expect.md renamed
compiler/forget/src/__tests__/fixtures/hir/while-logical.js new
+8
@@ -0,0 +1,8 @@
1 +// @skip
2 +function foo(props) {
3 + let x = 0;
4 + while (x > props.min && x < props.max) {
5 + x *= 2;
6 + }
7 + return x;
8 +}