Fix unused logical/condition via ExpressionStatement instr
Uses the new ExpressionStatement instruction to ensure that logical and conditional expressions are never pruned. This addresses an issue where we were unable to construct a ReactiveFunction for unused logical/conditional bc there wasn't a single Identifier assigned in both branches. The ExpressionStatement ensures that the result is used, that we don't prune the phi, and that both branches have a single assignment target. In theory we could be more sophisticated with DCE and still prune these instructions if their operands are also safe to prune, but in practice you're only like to have a logical/conditional as an expression statement (in the source) if it's for side effects.
Joe Savona committed
Apr 26, 2023 at 11:27 UTC
1f51b10ac9762550574485028a4081902d6bd034
6 files changed
+50
-25
compiler/forget/src/HIR/BuildHIR.ts
+18
-1
@@ -674,7 +674,24 @@ function lowerStatement(
674
case "ExpressionStatement": {
675
const stmt = stmtPath as NodePath<t.ExpressionStatement>;
676
const expression = stmt.get("expression");
677
- lowerExpressionToTemporary(builder, expression);
677
+ const value = lowerExpressionToTemporary(builder, expression);
678
+ const exprNode = expression.node;
679
+ if (
680
+ exprNode.type === "LogicalExpression" ||
681
+ exprNode.type === "ConditionalExpression"
682
+ ) {
683
+ const loc = exprNode.loc ?? GeneratedSource;
684
+ builder.push({
685
+ id: makeInstructionId(0),
686
+ lvalue: buildTemporaryPlace(builder, loc),
687
+ value: {
688
+ kind: "ExpressionStatement",
689
+ value,
690
+ loc,
691
+ },
692
+ loc,
693
+ });
694
+ }
695
return;
696
}
697
case "DoWhileStatement": {
compiler/forget/src/__tests__/fixtures/compiler/error.todo-unused-conditional.expect.md
deleted
-20
@@ -1,20 +0,0 @@
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/unused-conditional.expect.md
new
+24
@@ -0,0 +1,24 @@
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
+## Code
14
+
15
+```javascript
16
+function Component(props) {
17
+ let x = undefined;
18
+
19
+ ((x = 1), 1) && (x = 2);
20
+ return x;
21
+}
22
+
23
+```
24
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/unused-conditional.js
renamed
compiler/forget/src/__tests__/fixtures/compiler/unused-logical.expect.md
renamed
+8
-4
@@ -10,11 +10,15 @@ function Component(props) {
10
11
```
12
13
+## Code
14
14
-## Error
15
+```javascript
16
+function Component(props) {
17
+ let x = undefined;
18
+
19
+ props.cond ? (x = 1) : (x = 2);
20
+ return x;
21
+}
22
23
```
17
-[ReactForget] Todo: TODO: Support ConditionalExpression whose value is unused (3:3)
18
-```
19
-
24
\ No newline at end of file