From 9200ad027d91cc06dcd53167955b7e9ba04f5bd3 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 15 Mar 2023 09:39:23 -0700 Subject: [PATCH] Throw CompilerError (todo) for unused conditional/logical If a logical or conditional expression is unused, then a phi node isn't created for the identifier it assigns to. Then when we leave SSA form the two branches will assign to separate values, and we aren't sure which identifier to use as the lvalue of the resulting ReactiveInstruction (remember that logicals/conditionals decompose into control flow in HIR, but are a single compound instruction in ReactiveFunction). If the two sides don't assign to the same location, it could be because of a bug in the compiler or because the value wasn't used. Ideally we'd represent this explicitly, but for now i'm just making this a TODO since most logicals/conditionals should have their value used. --- compiler/forget/src/CompilerError.ts | 13 +++++++++++ .../ReactiveScopes/BuildReactiveFunction.ts | 22 ++++++++++++------- .../error.todo-unused-conditional.expect.md | 20 +++++++++++++++++ .../compiler/error.todo-unused-conditional.js | 5 +++++ .../error.todo-unused-logical.expect.md | 20 +++++++++++++++++ .../compiler/error.todo-unused-logical.js | 5 +++++ 6 files changed, 77 insertions(+), 8 deletions(-) create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-conditional.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-conditional.js create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-logical.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-logical.js diff --git a/compiler/forget/src/CompilerError.ts b/compiler/forget/src/CompilerError.ts index af3343fc45..c725ae20bd 100644 --- a/compiler/forget/src/CompilerError.ts +++ b/compiler/forget/src/CompilerError.ts @@ -115,6 +115,19 @@ export class CompilerError extends Error { throw errors; } + static todo(reason: string, loc: SourceLocation): never { + const errors = new CompilerError(); + errors.pushErrorDetail( + new CompilerErrorDetail({ + codeframe: null, + loc: typeof loc === "symbol" ? null : loc, + reason, + severity: ErrorSeverity.Todo, + }) + ); + throw errors; + } + constructor(...args: any[]) { super(...args); } diff --git a/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts b/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts index ca8b0757c0..a775633b06 100644 --- a/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts +++ b/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts @@ -6,6 +6,7 @@ */ import invariant from "invariant"; +import { CompilerError } from "../CompilerError"; import { BasicBlock, BlockId, @@ -681,10 +682,12 @@ class Driver { testBlock.terminal.alternate, terminal.loc ); - invariant( - leftFinal.place.identifier === right.place.identifier, - "Expected the left and right side of a logical expression to store a value to the same place" - ); + if (leftFinal.place.identifier !== right.place.identifier) { + CompilerError.todo( + "TODO: Support LogicalExpression whose value is unused", + leftFinal.place.loc + ); + } const value: ReactiveLogicalValue = { kind: "LogicalExpression", operator: terminal.operator, @@ -722,10 +725,13 @@ class Driver { alternate: alternate.value, loc: terminal.loc, }; - invariant( - consequent.place.identifier === alternate.place.identifier, - "Expected the consequent and alternate of a ternary to store a value to the same place" - ); + if (consequent.place.identifier !== alternate.place.identifier) { + CompilerError.todo( + "TODO: Support ConditionalExpression whose value is unused", + consequent.place.loc + ); + } + return { place: { ...consequent.place }, value, diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-conditional.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-conditional.expect.md new file mode 100644 index 0000000000..002d7a1161 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-conditional.expect.md @@ -0,0 +1,20 @@ + +## Input + +```javascript +function Component(props) { + let x = 0; + (x = 1) && (x = 2); + return x; +} + +``` + + +## Error + +``` +[ReactForget] Todo: TODO: Support LogicalExpression whose value is unused (3:3) +``` + + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-conditional.js b/compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-conditional.js new file mode 100644 index 0000000000..93197e33f7 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-conditional.js @@ -0,0 +1,5 @@ +function Component(props) { + let x = 0; + (x = 1) && (x = 2); + return x; +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-logical.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-logical.expect.md new file mode 100644 index 0000000000..795ca54c3f --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-logical.expect.md @@ -0,0 +1,20 @@ + +## Input + +```javascript +function Component(props) { + let x = 0; + props.cond ? (x = 1) : (x = 2); + return x; +} + +``` + + +## Error + +``` +[ReactForget] Todo: TODO: Support ConditionalExpression whose value is unused (3:3) +``` + + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-logical.js b/compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-logical.js new file mode 100644 index 0000000000..e4639a1611 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-logical.js @@ -0,0 +1,5 @@ +function Component(props) { + let x = 0; + props.cond ? (x = 1) : (x = 2); + return x; +}