From ef34ca6cb044bca24890464e52a2e04b1984ba1b Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Tue, 21 Mar 2023 10:01:09 -0700 Subject: [PATCH] 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. --- compiler/forget/src/HIR/BuildHIR.ts | 50 ++++++++++++------- .../ReactiveScopes/CodegenReactiveFunction.ts | 12 +++++ .../compiler/call-args-assignment.expect.md | 30 +++++++++++ .../fixtures/compiler/call-args-assignment.js | 5 ++ ...ll-args-destructuring-assignment.expect.md | 30 +++++++++++ .../call-args-destructuring-assignment.js | 5 ++ ...-destructuring-asignment-complex.expect.md | 20 ++++++++ ...ll-args-destructuring-asignment-complex.js | 5 ++ 8 files changed, 139 insertions(+), 18 deletions(-) create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/call-args-assignment.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/call-args-assignment.js create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/call-args-destructuring-assignment.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/call-args-destructuring-assignment.js create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/error.call-args-destructuring-asignment-complex.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/error.call-args-destructuring-asignment-complex.js diff --git a/compiler/forget/src/HIR/BuildHIR.ts b/compiler/forget/src/HIR/BuildHIR.ts index d30354b2ef..46f5d1bd92 100644 --- a/compiler/forget/src/HIR/BuildHIR.ts +++ b/compiler/forget/src/HIR/BuildHIR.ts @@ -2029,13 +2029,20 @@ function lowerAssignment( }); return { kind: "UnsupportedNode", node: lvalueNode, loc }; } - return { - kind: "PropertyStore", - object, - property: property.node.name, - value, + const temporary = buildTemporaryPlace(builder, loc); + builder.push({ + id: makeInstructionId(0), + lvalue: { ...temporary }, + value: { + kind: "PropertyStore", + object, + property: property.node.name, + value, + loc, + }, loc, - }; + }); + return { kind: "LoadLocal", place: temporary, loc: temporary.loc }; } else { if (!property.isExpression()) { builder.errors.push({ @@ -2047,13 +2054,20 @@ function lowerAssignment( return { kind: "UnsupportedNode", node: lvalueNode, loc }; } const propertyPlace = lowerExpressionToTemporary(builder, property); - return { - kind: "ComputedStore", - object, - property: propertyPlace, - value, + const temporary = buildTemporaryPlace(builder, loc); + builder.push({ + id: makeInstructionId(0), + lvalue: { ...temporary }, + value: { + kind: "ComputedStore", + object, + property: propertyPlace, + value, + loc, + }, loc, - }; + }); + return { kind: "LoadLocal", place: temporary, loc: temporary.loc }; } } case "ArrayPattern": { @@ -2093,10 +2107,10 @@ function lowerAssignment( followups.push({ place: temp, path: element as NodePath }); // TODO remove type cast } } - const temp = buildTemporaryPlace(builder, loc); + const temporary = buildTemporaryPlace(builder, loc); builder.push({ id: makeInstructionId(0), - lvalue: { ...temp }, + lvalue: { ...temporary }, value: { kind: "Destructure", lvalue: { @@ -2114,7 +2128,7 @@ function lowerAssignment( for (const { place, path } of followups) { lowerAssignment(builder, path.node.loc ?? loc, kind, path, place); } - return { kind: "LoadLocal", place: value, loc: value.loc }; + return { kind: "LoadLocal", place: temporary, loc: value.loc }; } case "ObjectPattern": { const lvalue = lvaluePath as NodePath; @@ -2187,10 +2201,10 @@ function lowerAssignment( } } } - const temp = buildTemporaryPlace(builder, loc); + const temporary = buildTemporaryPlace(builder, loc); builder.push({ id: makeInstructionId(0), - lvalue: { ...temp }, + lvalue: { ...temporary }, value: { kind: "Destructure", lvalue: { @@ -2208,7 +2222,7 @@ function lowerAssignment( for (const { place, path } of followups) { lowerAssignment(builder, path.node.loc ?? loc, kind, path, place); } - return { kind: "LoadLocal", place: value, loc: value.loc }; + return { kind: "LoadLocal", place: temporary, loc: value.loc }; } default: { builder.errors.push({ diff --git a/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts index 5970bfa960..c144c8a3b4 100644 --- a/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -393,11 +393,23 @@ function codegenInstructionNullable( } switch (kind) { case InstructionKind.Const: { + if (instr.lvalue !== null) { + CompilerError.invariant( + `Const declaration cannot be referenced as an expression`, + instr.value.loc + ); + } return createVariableDeclaration(instr.loc, "const", [ t.variableDeclarator(codegenLValue(lvalue), value), ]); } case InstructionKind.Let: { + if (instr.lvalue !== null) { + CompilerError.invariant( + `Const declaration cannot be referenced as an expression`, + instr.value.loc + ); + } return createVariableDeclaration(instr.loc, "let", [ t.variableDeclarator(codegenLValue(lvalue), value), ]); diff --git a/compiler/forget/src/__tests__/fixtures/compiler/call-args-assignment.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/call-args-assignment.expect.md new file mode 100644 index 0000000000..7c9fcbbad9 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/call-args-assignment.expect.md @@ -0,0 +1,30 @@ + +## Input + +```javascript +function Component(props) { + let x = makeObject(); + x.foo((x = makeObject())); + return x; +} + +``` + +## Code + +```javascript +function Component(props) { + const $ = React.unstable_useMemoCache(1); + let x; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + x = makeObject(); + x.foo((x = makeObject())); + $[0] = x; + } else { + x = $[0]; + } + return x; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/call-args-assignment.js b/compiler/forget/src/__tests__/fixtures/compiler/call-args-assignment.js new file mode 100644 index 0000000000..dc686eea9e --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/call-args-assignment.js @@ -0,0 +1,5 @@ +function Component(props) { + let x = makeObject(); + x.foo((x = makeObject())); + return x; +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/call-args-destructuring-assignment.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/call-args-destructuring-assignment.expect.md new file mode 100644 index 0000000000..3a7948ba5f --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/call-args-destructuring-assignment.expect.md @@ -0,0 +1,30 @@ + +## Input + +```javascript +function Component(props) { + let x = makeObject(); + x.foo(([x] = makeObject())); + return x; +} + +``` + +## Code + +```javascript +function Component(props) { + const $ = React.unstable_useMemoCache(1); + let x; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + x = makeObject(); + x.foo(([x] = makeObject())); + $[0] = x; + } else { + x = $[0]; + } + return x; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/call-args-destructuring-assignment.js b/compiler/forget/src/__tests__/fixtures/compiler/call-args-destructuring-assignment.js new file mode 100644 index 0000000000..e87fb49c34 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/call-args-destructuring-assignment.js @@ -0,0 +1,5 @@ +function Component(props) { + let x = makeObject(); + x.foo(([x] = makeObject())); + return x; +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.call-args-destructuring-asignment-complex.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/error.call-args-destructuring-asignment-complex.expect.md new file mode 100644 index 0000000000..14cfde8ff6 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.call-args-destructuring-asignment-complex.expect.md @@ -0,0 +1,20 @@ + +## Input + +```javascript +function Component(props) { + let x = makeObject(); + x.foo(([[x]] = makeObject())); + return x; +} + +``` + + +## Error + +``` +[ReactForget] Invariant: Const declaration cannot be referenced as an expression (3:3) +``` + + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.call-args-destructuring-asignment-complex.js b/compiler/forget/src/__tests__/fixtures/compiler/error.call-args-destructuring-asignment-complex.js new file mode 100644 index 0000000000..6f5dd4ea4a --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.call-args-destructuring-asignment-complex.js @@ -0,0 +1,5 @@ +function Component(props) { + let x = makeObject(); + x.foo(([[x]] = makeObject())); + return x; +}