@samitouri / QOS-React-2 / commits / ef34ca6cb0

Model other assignment variants as values

The previous PR only updated simple assignment expressions (where the lvalue is an identifier), this PR extends the same idea to all assignment variants. Note that there is one case that doesn't work yet, which is complex destructuring assignment as a value: ```javascript let x = makeObject(); x.foo(([[x]] = makeObject())); ``` What happens here is that we lower the destructuring to a series of steps: ``` tmp1: Destructure Const [ tmp0 ] = makeObject(); tmp2: Destructure Reassign [ x ] = tmp0; PropertyCall x, 'foo', [ tmp1 ] ``` Thankfully we can detect this case: if we have a const/let declaration with an lvalue, that's invalid. See the new error test case which shows we correctly detect & reject this case for now.

Joe Savona committed Mar 21, 2023 at 10:01 UTC ef34ca6cb044bca24890464e52a2e04b1984ba1b
8 files changed +139 -18
compiler/forget/src/HIR/BuildHIR.ts
+32 -18
@@ -2029,13 +2029,20 @@ function lowerAssignment(
2029 });
2030 return { kind: "UnsupportedNode", node: lvalueNode, loc };
2031 }
2032 - return {
2033 - kind: "PropertyStore",
2034 - object,
2035 - property: property.node.name,
2036 - value,
2032 + const temporary = buildTemporaryPlace(builder, loc);
2033 + builder.push({
2034 + id: makeInstructionId(0),
2035 + lvalue: { ...temporary },
2036 + value: {
2037 + kind: "PropertyStore",
2038 + object,
2039 + property: property.node.name,
2040 + value,
2041 + loc,
2042 + },
2043 loc,
2038 - };
2044 + });
2045 + return { kind: "LoadLocal", place: temporary, loc: temporary.loc };
2046 } else {
2047 if (!property.isExpression()) {
2048 builder.errors.push({
@@ -2047,13 +2054,20 @@ function lowerAssignment(
2054 return { kind: "UnsupportedNode", node: lvalueNode, loc };
2055 }
2056 const propertyPlace = lowerExpressionToTemporary(builder, property);
2050 - return {
2051 - kind: "ComputedStore",
2052 - object,
2053 - property: propertyPlace,
2054 - value,
2057 + const temporary = buildTemporaryPlace(builder, loc);
2058 + builder.push({
2059 + id: makeInstructionId(0),
2060 + lvalue: { ...temporary },
2061 + value: {
2062 + kind: "ComputedStore",
2063 + object,
2064 + property: propertyPlace,
2065 + value,
2066 + loc,
2067 + },
2068 loc,
2056 - };
2069 + });
2070 + return { kind: "LoadLocal", place: temporary, loc: temporary.loc };
2071 }
2072 }
2073 case "ArrayPattern": {
@@ -2093,10 +2107,10 @@ function lowerAssignment(
2107 followups.push({ place: temp, path: element as NodePath<t.LVal> }); // TODO remove type cast
2108 }
2109 }
2096 - const temp = buildTemporaryPlace(builder, loc);
2110 + const temporary = buildTemporaryPlace(builder, loc);
2111 builder.push({
2112 id: makeInstructionId(0),
2099 - lvalue: { ...temp },
2113 + lvalue: { ...temporary },
2114 value: {
2115 kind: "Destructure",
2116 lvalue: {
@@ -2114,7 +2128,7 @@ function lowerAssignment(
2128 for (const { place, path } of followups) {
2129 lowerAssignment(builder, path.node.loc ?? loc, kind, path, place);
2130 }
2117 - return { kind: "LoadLocal", place: value, loc: value.loc };
2131 + return { kind: "LoadLocal", place: temporary, loc: value.loc };
2132 }
2133 case "ObjectPattern": {
2134 const lvalue = lvaluePath as NodePath<t.ObjectPattern>;
@@ -2187,10 +2201,10 @@ function lowerAssignment(
2201 }
2202 }
2203 }
2190 - const temp = buildTemporaryPlace(builder, loc);
2204 + const temporary = buildTemporaryPlace(builder, loc);
2205 builder.push({
2206 id: makeInstructionId(0),
2193 - lvalue: { ...temp },
2207 + lvalue: { ...temporary },
2208 value: {
2209 kind: "Destructure",
2210 lvalue: {
@@ -2208,7 +2222,7 @@ function lowerAssignment(
2222 for (const { place, path } of followups) {
2223 lowerAssignment(builder, path.node.loc ?? loc, kind, path, place);
2224 }
2211 - return { kind: "LoadLocal", place: value, loc: value.loc };
2225 + return { kind: "LoadLocal", place: temporary, loc: value.loc };
2226 }
2227 default: {
2228 builder.errors.push({
compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+12
@@ -393,11 +393,23 @@ function codegenInstructionNullable(
393 }
394 switch (kind) {
395 case InstructionKind.Const: {
396 + if (instr.lvalue !== null) {
397 + CompilerError.invariant(
398 + `Const declaration cannot be referenced as an expression`,
399 + instr.value.loc
400 + );
401 + }
402 return createVariableDeclaration(instr.loc, "const", [
403 t.variableDeclarator(codegenLValue(lvalue), value),
404 ]);
405 }
406 case InstructionKind.Let: {
407 + if (instr.lvalue !== null) {
408 + CompilerError.invariant(
409 + `Const declaration cannot be referenced as an expression`,
410 + instr.value.loc
411 + );
412 + }
413 return createVariableDeclaration(instr.loc, "let", [
414 t.variableDeclarator(codegenLValue(lvalue), value),
415 ]);
compiler/forget/src/__tests__/fixtures/compiler/call-args-assignment.expect.md new
+30
@@ -0,0 +1,30 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component(props) {
6 + let x = makeObject();
7 + x.foo((x = makeObject()));
8 + return x;
9 +}
10 +
11 +```
12 +
13 +## Code
14 +
15 +```javascript
16 +function Component(props) {
17 + const $ = React.unstable_useMemoCache(1);
18 + let x;
19 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
20 + x = makeObject();
21 + x.foo((x = makeObject()));
22 + $[0] = x;
23 + } else {
24 + x = $[0];
25 + }
26 + return x;
27 +}
28 +
29 +```
30 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/call-args-assignment.js new
+5
@@ -0,0 +1,5 @@
1 +function Component(props) {
2 + let x = makeObject();
3 + x.foo((x = makeObject()));
4 + return x;
5 +}
compiler/forget/src/__tests__/fixtures/compiler/call-args-destructuring-assignment.expect.md new
+30
@@ -0,0 +1,30 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component(props) {
6 + let x = makeObject();
7 + x.foo(([x] = makeObject()));
8 + return x;
9 +}
10 +
11 +```
12 +
13 +## Code
14 +
15 +```javascript
16 +function Component(props) {
17 + const $ = React.unstable_useMemoCache(1);
18 + let x;
19 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
20 + x = makeObject();
21 + x.foo(([x] = makeObject()));
22 + $[0] = x;
23 + } else {
24 + x = $[0];
25 + }
26 + return x;
27 +}
28 +
29 +```
30 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/call-args-destructuring-assignment.js new
+5
@@ -0,0 +1,5 @@
1 +function Component(props) {
2 + let x = makeObject();
3 + x.foo(([x] = makeObject()));
4 + return x;
5 +}
compiler/forget/src/__tests__/fixtures/compiler/error.call-args-destructuring-asignment-complex.expect.md new
+20
@@ -0,0 +1,20 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component(props) {
6 + let x = makeObject();
7 + x.foo(([[x]] = makeObject()));
8 + return x;
9 +}
10 +
11 +```
12 +
13 +
14 +## Error
15 +
16 +```
17 +[ReactForget] Invariant: Const declaration cannot be referenced as an expression (3:3)
18 +```
19 +
20 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/error.call-args-destructuring-asignment-complex.js new
+5
@@ -0,0 +1,5 @@
1 +function Component(props) {
2 + let x = makeObject();
3 + x.foo(([[x]] = makeObject()));
4 + return x;
5 +}