@samitouri / QOS-React-1 / commits / ade3235ca3

ForInStatement: specialize inference/types

Replaces the use of `NextIterableOf` in for-in with a new `NextPropertyOf` instruction. The key distinction is `for-of` invokes an arbitrary iterator, which means a) each iteration may mutate the collection being iterated and b) the returned value may be mutable. However, `for-in` invokes a language-level mechanism to iterate: simply iterating alone _cannot_ modify the collection, and the returned value is known to be a primitive.

Joe Savona committed Sep 11, 2023 at 16:54 UTC ade3235ca3053f8786579050dc4dffa48d10233f
12 files changed +104 -6
compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts
+4 -4
@@ -943,7 +943,7 @@ function lowerStatement(
943 initBlock
944 );
945
946 - // The init of a ForOf statement is compound over a left (VariableDeclaration | LVal) and
946 + // The init of a ForIn statement is compound over a left (VariableDeclaration | LVal) and
947 // right (Expression), so we synthesize a new InstrValue and assignment (potentially multiple
948 // instructions when we handle other syntax like Patterns)
949 const left = stmt.get("left");
@@ -952,14 +952,14 @@ function lowerStatement(
952 if (left.isVariableDeclaration()) {
953 const declarations = left.get("declarations");
954 CompilerError.invariant(declarations.length === 1, {
955 - reason: `Expected only one declaration in the init of a ForOfStatement, got ${declarations.length}`,
955 + reason: `Expected only one declaration in the init of a ForInStatement, got ${declarations.length}`,
956 description: null,
957 loc: left.node.loc ?? null,
958 suggestions: null,
959 });
960 const id = declarations[0].get("id");
961 const nextIterableOf = lowerValueToTemporary(builder, {
962 - kind: "NextIterableOf", // TODO: change this to reflect for-in semantics (returns immutable keys, does not modify collection)
962 + kind: "NextPropertyOf",
963 loc: leftLoc,
964 value,
965 });
@@ -973,7 +973,7 @@ function lowerStatement(
973 test = lowerValueToTemporary(builder, assign);
974 } else {
975 builder.errors.push({
976 - reason: `(BuildHIR::lowerStatement) Handle ${left.type} inits in ForOfStatement`,
976 + reason: `(BuildHIR::lowerStatement) Handle ${left.type} inits in ForInStatement`,
977 severity: ErrorSeverity.Todo,
978 loc: left.node.loc ?? null,
979 suggestions: null,
compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts
+5
@@ -788,6 +788,11 @@ export type InstructionValue =
788 value: Place; // the collection
789 loc: SourceLocation;
790 }
791 + | {
792 + kind: "NextPropertyOf";
793 + value: Place; // the collection
794 + loc: SourceLocation;
795 + }
796 // Models a prefix update expression such as --x or ++y
797 // This instructions increments or decrements the <lvalue>
798 // but evaluates to the value of <value> prior to the update.
compiler/packages/babel-plugin-react-forget/src/HIR/PrintHIR.ts
+4
@@ -542,6 +542,10 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
542 value = `NextIterableOf ${printPlace(instrValue.value)}`;
543 break;
544 }
545 + case "NextPropertyOf": {
546 + value = `NextPropertyOf ${printPlace(instrValue.value)}`;
547 + break;
548 + }
549 case "Debugger": {
550 value = `Debugger`;
551 break;
compiler/packages/babel-plugin-react-forget/src/HIR/visitors.ts
+8
@@ -193,6 +193,10 @@ export function* eachInstructionValueOperand(
193 yield instrValue.value;
194 break;
195 }
196 + case "NextPropertyOf": {
197 + yield instrValue.value;
198 + break;
199 + }
200 case "PostfixUpdate":
201 case "PrefixUpdate": {
202 yield instrValue.value;
@@ -476,6 +480,10 @@ export function mapInstructionOperands(
480 instrValue.value = fn(instrValue.value);
481 break;
482 }
483 + case "NextPropertyOf": {
484 + instrValue.value = fn(instrValue.value);
485 + break;
486 + }
487 case "PostfixUpdate":
488 case "PrefixUpdate": {
489 instrValue.value = fn(instrValue.value);
compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts
+6
@@ -1020,6 +1020,12 @@ function inferBlock(
1020 valueKind = ValueKind.Mutable;
1021 break;
1022 }
1023 + case "NextPropertyOf": {
1024 + effectKind = Effect.Read;
1025 + lvalueEffect = Effect.Store;
1026 + valueKind = ValueKind.Immutable;
1027 + break;
1028 + }
1029 default: {
1030 assertExhaustive(instrValue, "Unexpected instruction kind");
1031 }
compiler/packages/babel-plugin-react-forget/src/Optimization/DeadCodeElimination.ts
+4 -2
@@ -217,9 +217,11 @@ function pruneableValue(value: InstructionValue, state: State): boolean {
217 // Potentially safe to prune, since they should just be creating new values
218 return false;
219 }
220 + case "NextPropertyOf":
221 case "NextIterableOf": {
221 - // Technically a NextIterableOf will never be unused because it's always used later by
222 - // another StoreLocal or Destructure instruction, but conceptually we can't prune
222 + // Technically a NextIterableOf/NextPropertyOf will never be unused because it's
223 + // always used later by another StoreLocal or Destructure instruction, but conceptually
224 + // we can't prune
225 return false;
226 }
227 case "LoadContext":
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+4
@@ -1212,6 +1212,10 @@ function codegenInstructionValue(
1212 value = codegenPlace(cx, instrValue.value);
1213 break;
1214 }
1215 + case "NextPropertyOf": {
1216 + value = codegenPlace(cx, instrValue.value);
1217 + break;
1218 + }
1219 case "PostfixUpdate": {
1220 value = t.updateExpression(
1221 instrValue.operation,
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts
+1
@@ -244,6 +244,7 @@ function mayAllocate(env: Environment, instruction: Instruction): boolean {
244 case "TemplateLiteral":
245 case "Primitive":
246 case "NextIterableOf":
247 + case "NextPropertyOf":
248 case "Debugger": {
249 return false;
250 }
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts
+1
@@ -430,6 +430,7 @@ function computeMemoizationInputs(
430 rvalues: value.children,
431 };
432 }
433 + case "NextPropertyOf":
434 case "Debugger":
435 case "ComputedDelete":
436 case "PropertyDelete":
compiler/packages/babel-plugin-react-forget/src/TypeInference/InferTypes.ts
+5
@@ -260,6 +260,11 @@ function* generateInstructionTypes(
260 break;
261 }
262
263 + case "NextPropertyOf": {
264 + yield equation(left, { kind: "Primitive" });
265 + break;
266 + }
267 +
268 case "DeclareLocal":
269 case "NewExpression":
270 case "JsxExpression":
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-in-statement-type-inference.expect.md new
+46
@@ -0,0 +1,46 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +const { identity, mutate } = require("shared-runtime");
6 +
7 +function Component(props) {
8 + let x;
9 + const object = { ...props.value };
10 + for (const y in object) {
11 + x = y;
12 + }
13 + mutate(x); // can't modify, x is known primitive!
14 + return x;
15 +}
16 +
17 +export const FIXTURE_ENTRYPOINT = {
18 + fn: Component,
19 + params: [{ value: { a: "a", b: "B", c: "C!" } }],
20 +};
21 +
22 +```
23 +
24 +## Code
25 +
26 +```javascript
27 +const { identity, mutate } = require("shared-runtime");
28 +
29 +function Component(props) {
30 + let x;
31 + const object = { ...props.value };
32 + for (const y in object) {
33 + x = y;
34 + }
35 +
36 + mutate(x);
37 + return x;
38 +}
39 +
40 +export const FIXTURE_ENTRYPOINT = {
41 + fn: Component,
42 + params: [{ value: { a: "a", b: "B", c: "C!" } }],
43 +};
44 +
45 +```
46 +
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-in-statement-type-inference.js new
+16
@@ -0,0 +1,16 @@
1 +const { identity, mutate } = require("shared-runtime");
2 +
3 +function Component(props) {
4 + let x;
5 + const object = { ...props.value };
6 + for (const y in object) {
7 + x = y;
8 + }
9 + mutate(x); // can't modify, x is known primitive!
10 + return x;
11 +}
12 +
13 +export const FIXTURE_ENTRYPOINT = {
14 + fn: Component,
15 + params: [{ value: { a: "a", b: "B", c: "C!" } }],
16 +};