[hir] Allow reorderable exprs in optional computed load
Mofei Zhang committed
Mar 31, 2023 at 10:58 UTC
8dfe06ddba1abbb6872b5e4384b3aa3485e86217
8 files changed
+65
-23
compiler/forget/src/HIR/BuildHIR.ts
+21
-12
@@ -1680,9 +1680,11 @@ function lowerExpression(
1680
}
1681
1682
/**
1683
- * There are a few places where we do not preserve original evaluation ordering, such as switch case test values
1684
- * and default values in destructuring (assignment patterns). In these cases we allow simple expressions whose
1685
- * evaluation cannot be observed: primitives and arrays/objects whose values are also safely reorderable.
1683
+ * There are a few places where we do not preserve original evaluation ordering and/or control flow, such as
1684
+ * switch case test values and default values in destructuring (assignment patterns). In these cases we allow
1685
+ * simple expressions whose evaluation cannot be observed:
1686
+ * - primitives
1687
+ * - arrays/objects whose values are also safely reorderable.
1688
*/
1689
function lowerReorderableExpression(
1690
builder: HIRBuilder,
@@ -1864,21 +1866,28 @@ function lowerMemberExpression(
1866
},
1867
};
1868
}
1867
- if (t.isOptionalMemberExpression(expr)) {
1868
- builder.errors.push({
1869
- reason: `(BuildHIR::lowerMemberExpression) Handle computed OptionalMemberExpression`,
1870
- severity: ErrorSeverity.Todo,
1871
- nodePath: expr,
1872
- });
1869
+ let optional;
1870
+ let property: Place;
1871
+
1872
+ // See "PropertyLoad" for the difference between optionalMemberExpr()
1873
+ // and node.optional here
1874
+ if (expr.isOptionalMemberExpression()) {
1875
+ // if expr is in an optional chain, evaluation of `property` is
1876
+ // conditional on whether expr is nullish
1877
+ property = lowerReorderableExpression(builder, propertyNode);
1878
+ optional = expr.node.optional ?? false;
1879
+ } else {
1880
+ property = lowerExpressionToTemporary(builder, propertyNode);
1881
+ optional = false;
1882
}
1874
- const propertyPlace = lowerExpressionToTemporary(builder, propertyNode);
1883
const value: InstructionValue = {
1884
kind: "ComputedLoad",
1885
object: { ...object },
1878
- property: { ...propertyPlace },
1886
+ property: { ...property },
1887
loc: exprLoc,
1888
+ optional,
1889
};
1881
- return { object, property: propertyPlace, value };
1890
+ return { object, property, value };
1891
}
1892
}
1893
compiler/forget/src/HIR/HIR.ts
+1
@@ -609,6 +609,7 @@ export type InstructionValue =
609
kind: "ComputedLoad";
610
object: Place;
611
property: Place;
612
+ optional: boolean;
613
loc: SourceLocation;
614
}
615
// `delete object[property]`
compiler/forget/src/HIR/PrintHIR.ts
+3
-3
@@ -367,9 +367,9 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
367
break;
368
}
369
case "ComputedLoad": {
370
- value = `ComputedLoad ${printPlace(instrValue.object)}[${printPlace(
371
- instrValue.property
372
- )}]`;
370
+ value = `ComputedLoad ${printPlace(instrValue.object)}${
371
+ instrValue.optional ? "?" : ""
372
+ }[${printPlace(instrValue.property)}]`;
373
break;
374
}
375
case "ComputedStore": {
compiler/forget/src/Optimization/ConstantPropagation.ts
+1
-1
@@ -160,7 +160,7 @@ function evaluateInstruction(
160
loc: value.loc,
161
property: property.value,
162
object: value.object,
163
- optional: false,
163
+ optional: value.optional,
164
};
165
// Future-proofing: when we add support for optional computed properties,
166
// we'll need to copy the value here
compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+12
-5
@@ -795,11 +795,18 @@ function codegenInstructionValue(
795
break;
796
}
797
case "ComputedLoad": {
798
- value = t.memberExpression(
799
- codegenPlace(cx, instrValue.object),
800
- codegenPlace(cx, instrValue.property),
801
- true
802
- );
798
+ const object = codegenPlace(cx, instrValue.object);
799
+ const property = codegenPlace(cx, instrValue.property);
800
+ if (t.isOptionalMemberExpression(object) || instrValue.optional) {
801
+ value = t.optionalMemberExpression(
802
+ object,
803
+ property,
804
+ true,
805
+ instrValue.optional
806
+ );
807
+ } else {
808
+ value = t.memberExpression(object, property, true, instrValue.optional);
809
+ }
810
break;
811
}
812
case "ComputedDelete": {
compiler/forget/src/__tests__/fixtures/compiler/error.optional-computed-member-expression.expect.md
+2
-2
@@ -13,11 +13,11 @@ function Component(props) {
13
## Error
14
15
```
16
-[ReactForget] TodoError: (BuildHIR::lowerMemberExpression) Handle computed OptionalMemberExpression
16
+[ReactForget] TodoError: (BuildHIR::node.lowerReorderableExpression) Expression type 'MemberExpression' cannot be safely reordered
17
1 | function Component(props) {
18
2 | const object = makeObject(props);
19
> 3 | return object?.[props.key];
20
- | ^^^^^^^^^^^^^^^^^^^
20
+ | ^^^^^^^^^
21
4 | }
22
5 |
23
```
compiler/forget/src/__tests__/fixtures/compiler/optional-computed-load-static.expect.md
new
+21
@@ -0,0 +1,21 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ let x = a?.b.c[0];
7
+ return x;
8
+}
9
+
10
+```
11
+
12
+## Code
13
+
14
+```javascript
15
+function Component(props) {
16
+ const x = a?.b.c[0];
17
+ return x;
18
+}
19
+
20
+```
21
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/optional-computed-load-static.js
new
+4
@@ -0,0 +1,4 @@
1
+function Component(props) {
2
+ let x = a?.b.c[0];
3
+ return x;
4
+}