Skip to content

Commit

Permalink
[compiler] Fix assignment within for update expression
Browse files Browse the repository at this point in the history
When converting value blocks from HIR to ReactiveFunction, we have to drop StoreLocal assignments that represent the assignment of the phi, since ReactiveFunction supports compound expressions. These StoreLocals are only present to represent the conditional assignment of the value itself - but it's also possible for the expression to have contained an assignment expression. Before, in trying to strip the first category of StoreLocal we also accidentally stripped the second category. Now we check that the assignment is for a temporary, and don't strip otherwise.

ghstack-source-id: cdb81880f29d5be3313086bd9e3fcfef9db0f85e
Pull Request resolved: #30067
  • Loading branch information
josephsavona committed Jun 24, 2024
1 parent c21bcd6 commit 48b0da7
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -921,14 +921,17 @@ class Driver {
});
} else if (defaultBlock.instructions.length === 1) {
const instr = defaultBlock.instructions[0]!;
let place: Place = instr.lvalue!;
let place: Place = instr.lvalue;
let value: ReactiveValue = instr.value;
if (instr.value.kind === "StoreLocal") {
place = instr.value.lvalue.place;
if (
value.kind === "StoreLocal" &&
value.lvalue.place.identifier.name === null
) {
place = value.lvalue.place;
value = {
kind: "LoadLocal",
place: instr.value.value,
loc: instr.value.value.loc,
place: value.value,
loc: value.value.loc,
};
}
return {
Expand All @@ -939,14 +942,17 @@ class Driver {
};
} else {
const instr = defaultBlock.instructions.at(-1)!;
let place: Place = instr.lvalue!;
let place: Place = instr.lvalue;
let value: ReactiveValue = instr.value;
if (instr.value.kind === "StoreLocal") {
place = instr.value.lvalue.place;
if (
value.kind === "StoreLocal" &&
value.lvalue.place.identifier.name === null
) {
place = value.lvalue.place;
value = {
kind: "LoadLocal",
place: instr.value.value,
loc: instr.value.value.loc,
place: value.value,
loc: value.value.loc,
};
}
const sequence: ReactiveSequenceValue = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@

## Input

```javascript
function Component(props) {
let x = props.init;
for (let i = 0; i < 100; i = i + 1) {
x += i;
}
return [x];
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ init: 0 }],
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime";
function Component(props) {
const $ = _c(2);
let x = props.init;
for (let i = 0; i < 100; i = i + 1) {
x = x + i;
}
let t0;
if ($[0] !== x) {
t0 = [x];
$[0] = x;
$[1] = t0;
} else {
t0 = $[1];
}
return t0;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ init: 0 }],
};

```
### Eval output
(kind: ok) [4950]
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
function Component(props) {
let x = props.init;
for (let i = 0; i < 100; i = i + 1) {
x += i;
}
return [x];
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ init: 0 }],
};

0 comments on commit 48b0da7

Please sign in to comment.