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.
Joe Savona committed
Mar 15, 2023 at 09:39 UTC
9200ad027d91cc06dcd53167955b7e9ba04f5bd3
6 files changed
+77
-8
compiler/forget/src/CompilerError.ts
+13
@@ -115,6 +115,19 @@ export class CompilerError extends Error {
115
throw errors;
116
}
117
118
+ static todo(reason: string, loc: SourceLocation): never {
119
+ const errors = new CompilerError();
120
+ errors.pushErrorDetail(
121
+ new CompilerErrorDetail({
122
+ codeframe: null,
123
+ loc: typeof loc === "symbol" ? null : loc,
124
+ reason,
125
+ severity: ErrorSeverity.Todo,
126
+ })
127
+ );
128
+ throw errors;
129
+ }
130
+
131
constructor(...args: any[]) {
132
super(...args);
133
}
compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts
+14
-8
@@ -6,6 +6,7 @@
6
*/
7
8
import invariant from "invariant";
9
+import { CompilerError } from "../CompilerError";
10
import {
11
BasicBlock,
12
BlockId,
@@ -681,10 +682,12 @@ class Driver {
682
testBlock.terminal.alternate,
683
terminal.loc
684
);
684
- invariant(
685
- leftFinal.place.identifier === right.place.identifier,
686
- "Expected the left and right side of a logical expression to store a value to the same place"
687
- );
685
+ if (leftFinal.place.identifier !== right.place.identifier) {
686
+ CompilerError.todo(
687
+ "TODO: Support LogicalExpression whose value is unused",
688
+ leftFinal.place.loc
689
+ );
690
+ }
691
const value: ReactiveLogicalValue = {
692
kind: "LogicalExpression",
693
operator: terminal.operator,
@@ -722,10 +725,13 @@ class Driver {
725
alternate: alternate.value,
726
loc: terminal.loc,
727
};
725
- invariant(
726
- consequent.place.identifier === alternate.place.identifier,
727
- "Expected the consequent and alternate of a ternary to store a value to the same place"
728
- );
728
+ if (consequent.place.identifier !== alternate.place.identifier) {
729
+ CompilerError.todo(
730
+ "TODO: Support ConditionalExpression whose value is unused",
731
+ consequent.place.loc
732
+ );
733
+ }
734
+
735
return {
736
place: { ...consequent.place },
737
value,
compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-conditional.expect.md
new
+20
@@ -0,0 +1,20 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ let x = 0;
7
+ (x = 1) && (x = 2);
8
+ return x;
9
+}
10
+
11
+```
12
+
13
+
14
+## Error
15
+
16
+```
17
+[ReactForget] Todo: TODO: Support LogicalExpression whose value is unused (3:3)
18
+```
19
+
20
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-conditional.js
new
+5
@@ -0,0 +1,5 @@
1
+function Component(props) {
2
+ let x = 0;
3
+ (x = 1) && (x = 2);
4
+ return x;
5
+}
compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-logical.expect.md
new
+20
@@ -0,0 +1,20 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ let x = 0;
7
+ props.cond ? (x = 1) : (x = 2);
8
+ return x;
9
+}
10
+
11
+```
12
+
13
+
14
+## Error
15
+
16
+```
17
+[ReactForget] Todo: TODO: Support ConditionalExpression whose value is unused (3:3)
18
+```
19
+
20
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-logical.js
new
+5
@@ -0,0 +1,5 @@
1
+function Component(props) {
2
+ let x = 0;
3
+ props.cond ? (x = 1) : (x = 2);
4
+ return x;
5
+}