Remove unnecessary `optional` property on loads
Now that _all_ optional expression types use the new representation, the optionality of all PropertyLoad and ComputedLoad is modeled via control flow (in HIR) and the structure of OptionalExpression (in ReactiveFunction). Thus we no longer need the `optional` properties on these load instructions — they're optional if they're part of an OptionalExpression.
Joe Savona committed
May 3, 2023 at 17:10 UTC
e192930cd4b4d8e19627db1c33dbc313767f03d8
6 files changed
+18
-66
compiler/forget/src/HIR/BuildHIR.ts
-7
@@ -2157,7 +2157,6 @@ function lowerMemberExpression(
2157
object: { ...object },
2158
property: propertyNode.node.name,
2159
loc: exprLoc,
2160
- optional: expr.node.optional ?? false,
2160
};
2161
return { object, property: propertyNode.node.name, value };
2162
} else {
@@ -2177,26 +2176,21 @@ function lowerMemberExpression(
2176
},
2177
};
2178
}
2180
- let optional;
2179
let property: Place;
2182
-
2180
// See "PropertyLoad" for the difference between optionalMemberExpr()
2181
// and node.optional here
2182
if (expr.isOptionalMemberExpression()) {
2183
// if expr is in an optional chain, evaluation of `property` is
2184
// conditional on whether expr is nullish
2185
property = lowerReorderableExpression(builder, propertyNode);
2189
- optional = expr.node.optional ?? false;
2186
} else {
2187
property = lowerExpressionToTemporary(builder, propertyNode);
2192
- optional = false;
2188
}
2189
const value: InstructionValue = {
2190
kind: "ComputedLoad",
2191
object: { ...object },
2192
property: { ...property },
2193
loc: exprLoc,
2199
- optional,
2194
};
2195
return { object, property, value };
2196
}
@@ -2280,7 +2274,6 @@ function lowerJsxMemberExpression(
2274
kind: "PropertyLoad",
2275
object: objectPlace,
2276
property,
2283
- optional: false,
2277
loc,
2278
});
2279
}
compiler/forget/src/HIR/HIR.ts
-2
@@ -645,7 +645,6 @@ export type InstructionValue =
645
kind: "PropertyLoad";
646
object: Place;
647
property: string;
648
- optional: boolean;
648
loc: SourceLocation;
649
}
650
// `delete object.property`
@@ -669,7 +668,6 @@ export type InstructionValue =
668
kind: "ComputedLoad";
669
object: Place;
670
property: Place;
672
- optional: boolean;
671
loc: SourceLocation;
672
}
673
// `delete object[property]`
compiler/forget/src/HIR/PrintHIR.ts
+6
-6
@@ -367,9 +367,9 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
367
break;
368
}
369
case "PropertyLoad": {
370
- value = `PropertyLoad ${printPlace(instrValue.object)}${
371
- instrValue.optional ? "?" : ""
372
- }.${instrValue.property}`;
370
+ value = `PropertyLoad ${printPlace(instrValue.object)}.${
371
+ instrValue.property
372
+ }`;
373
break;
374
}
375
case "PropertyStore": {
@@ -385,9 +385,9 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
385
break;
386
}
387
case "ComputedLoad": {
388
- value = `ComputedLoad ${printPlace(instrValue.object)}${
389
- instrValue.optional ? "?" : ""
390
- }[${printPlace(instrValue.property)}]`;
388
+ value = `ComputedLoad ${printPlace(instrValue.object)}[${printPlace(
389
+ instrValue.property
390
+ )}]`;
391
break;
392
}
393
case "ComputedStore": {
compiler/forget/src/Optimization/ConstantPropagation.ts
-10
@@ -6,7 +6,6 @@
6
*/
7
8
import { isValidIdentifier } from "@babel/types";
9
-import invariant from "invariant";
9
import {
10
GotoVariant,
11
HIRFunction,
@@ -180,16 +179,7 @@ function evaluateInstruction(
179
loc: value.loc,
180
property: property.value,
181
object: value.object,
183
- optional: value.optional,
182
};
185
- // Future-proofing: when we add support for optional computed properties,
186
- // we'll need to copy the value here
187
- if ((value as any).optional) {
188
- invariant(
189
- false,
190
- "TODO: translate optional computed load to optional property load"
191
- );
192
- }
183
instr.value = nextValue;
184
}
185
return null;
compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+6
-25
@@ -853,21 +853,11 @@ function codegenInstructionValue(
853
const object = codegenPlace(cx, instrValue.object);
854
// We currently only lower single chains of optional memberexpr.
855
// (See BuildHIR.ts for more detail.)
856
- if (t.isOptionalMemberExpression(object) || instrValue.optional) {
857
- value = t.optionalMemberExpression(
858
- object,
859
- t.identifier(instrValue.property),
860
- undefined,
861
- instrValue.optional
862
- );
863
- } else {
864
- value = t.memberExpression(
865
- object,
866
- t.identifier(instrValue.property),
867
- undefined,
868
- instrValue.optional
869
- );
870
- }
856
+ value = t.memberExpression(
857
+ object,
858
+ t.identifier(instrValue.property),
859
+ undefined
860
+ );
861
break;
862
}
863
case "PropertyDelete": {
@@ -895,16 +885,7 @@ function codegenInstructionValue(
885
case "ComputedLoad": {
886
const object = codegenPlace(cx, instrValue.object);
887
const property = codegenPlace(cx, instrValue.property);
898
- if (t.isOptionalMemberExpression(object) || instrValue.optional) {
899
- value = t.optionalMemberExpression(
900
- object,
901
- property,
902
- true,
903
- instrValue.optional
904
- );
905
- } else {
906
- value = t.memberExpression(object, property, true, instrValue.optional);
907
- }
888
+ value = t.memberExpression(object, property, true);
889
break;
890
}
891
case "ComputedDelete": {
compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+6
-16
@@ -306,13 +306,8 @@ class Context {
306
return objectDependency;
307
}
308
309
- declareProperty(
310
- lvalue: Place,
311
- object: Place,
312
- property: string,
313
- isConditional: boolean
314
- ): void {
315
- const nextDependency = this.#getProperty(object, property, isConditional);
309
+ declareProperty(lvalue: Place, object: Place, property: string): void {
310
+ const nextDependency = this.#getProperty(object, property, false);
311
this.#properties.set(lvalue.identifier, nextDependency);
312
}
313
@@ -363,8 +358,8 @@ class Context {
358
this.visitDependency(dependency);
359
}
360
366
- visitProperty(object: Place, property: string, isConditional: boolean): void {
367
- const nextDependency = this.#getProperty(object, property, isConditional);
361
+ visitProperty(object: Place, property: string): void {
362
+ const nextDependency = this.#getProperty(object, property, false);
363
this.visitDependency(nextDependency);
364
}
365
@@ -529,14 +524,9 @@ class PropagationVisitor extends ReactiveFunctionVisitor<Context> {
524
}
525
} else if (value.kind === "PropertyLoad") {
526
if (lvalue !== null && !context.isUsedOutsideDeclaringScope(lvalue)) {
532
- context.declareProperty(
533
- lvalue,
534
- value.object,
535
- value.property,
536
- value.optional
537
- );
527
+ context.declareProperty(lvalue, value.object, value.property);
528
} else {
539
- context.visitProperty(value.object, value.property, value.optional);
529
+ context.visitProperty(value.object, value.property);
530
}
531
} else if (value.kind === "StoreLocal") {
532
context.visitOperand(value.value);