Skip to content

[compiler][ez] Patch Array.concat object shape to capture callee #30074

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 25, 2024
Merged

[compiler][ez] Patch Array.concat object shape to capture callee #30074

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1c55d88
[compiler][ez] Patch Array.concat object shape to capture callee
mofeiZ Jun 24, 2024
e724d4d
Update on "[compiler][ez] Patch Array.concat object shape to capture …
mofeiZ Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ addObject(BUILTIN_SHAPES, BuiltInArrayId, [
kind: "Object",
shapeId: BuiltInArrayId,
},
calleeEffect: Effect.Read,
calleeEffect: Effect.Capture,
Copy link
Member

@josephsavona josephsavona Jun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really want to redo our effects system, the current groupings are imprecise since it captures the content but not the array itself. But the tests cases look good so this is fine, just an aside.

returnValueKind: ValueKind.Mutable,
}),
],
Expand Down
24 changes: 10 additions & 14 deletions ...bug-array-concat-should-capture.expect.md → ...ler/array-concat-should-capture.expect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,17 @@ import { mutate } from "shared-runtime";
* itself.
*/
function Foo(t0) {
const $ = _c(3);
const $ = _c(2);
const { inputNum } = t0;
let t1;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t1 = [{ a: 1 }, {}];
$[0] = t1;
} else {
t1 = $[0];
}
const arr1 = t1;
let arr2;
if ($[1] !== inputNum) {
if ($[0] !== inputNum) {
const arr1 = [{ a: 1 }, {}];
arr2 = arr1.concat([1, inputNum]);
mutate(arr2[0]);
$[1] = inputNum;
$[2] = arr2;
$[0] = inputNum;
$[1] = arr2;
} else {
arr2 = $[2];
arr2 = $[1];
}
return arr2;
}
Expand All @@ -69,4 +62,7 @@ export const FIXTURE_ENTRYPOINT = {
};

```


### Eval output
(kind: ok) [{"a":1,"wat0":"joe"},{},1,2]
[{"a":1,"wat0":"joe"},{},1,3]
0 ...mpiler/bug-array-concat-should-capture.ts → ...s/compiler/array-concat-should-capture.ts
View file
Open in desktop
File renamed without changes.
61 changes: 27 additions & 34 deletions ...r/preserve-memo-validation/useCallback-reordering-deplist-controlflow.expect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,53 +40,46 @@ import { useCallback } from "react";
import { Stringify } from "shared-runtime";

function Foo(t0) {
const $ = _c(11);
const $ = _c(9);
const { arr1, arr2, foo } = t0;
let t1;
if ($[0] !== arr1) {
t1 = [arr1];
$[0] = arr1;
$[1] = t1;
} else {
t1 = $[1];
}
const x = t1;
let t2;
let getVal1;
if ($[2] !== foo || $[3] !== x || $[4] !== arr2) {
if ($[0] !== arr1 || $[1] !== foo || $[2] !== arr2) {
const x = [arr1];

let y;
y = [];
let t3;
if ($[7] === Symbol.for("react.memo_cache_sentinel")) {
t3 = () => ({ x: 2 });
$[7] = t3;
let t2;
if ($[5] === Symbol.for("react.memo_cache_sentinel")) {
t2 = () => ({ x: 2 });
$[5] = t2;
} else {
t3 = $[7];
t2 = $[5];
}
getVal1 = t3;
getVal1 = t2;

t2 = () => [y];
t1 = () => [y];
foo ? (y = x.concat(arr2)) : y;
$[2] = foo;
$[3] = x;
$[4] = arr2;
$[5] = t2;
$[6] = getVal1;
$[0] = arr1;
$[1] = foo;
$[2] = arr2;
$[3] = t1;
$[4] = getVal1;
} else {
t2 = $[5];
getVal1 = $[6];
t1 = $[3];
getVal1 = $[4];
}
const getVal2 = t2;
let t3;
if ($[8] !== getVal1 || $[9] !== getVal2) {
t3 = <Stringify val1={getVal1} val2={getVal2} shouldInvokeFns={true} />;
$[8] = getVal1;
$[9] = getVal2;
$[10] = t3;
const getVal2 = t1;
let t2;
if ($[6] !== getVal1 || $[7] !== getVal2) {
t2 = <Stringify val1={getVal1} val2={getVal2} shouldInvokeFns={true} />;
$[6] = getVal1;
$[7] = getVal2;
$[8] = t2;
} else {
t3 = $[10];
t2 = $[8];
}
return t3;
return t2;
}

export const FIXTURE_ENTRYPOINT = {
Expand Down
39 changes: 16 additions & 23 deletions ...r/preserve-memo-validation/useCallback-reordering-depslist-assignment.expect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,38 +36,31 @@ import { Stringify } from "shared-runtime";

// We currently produce invalid output (incorrect scoping for `y` declaration)
function useFoo(arr1, arr2) {
const $ = _c(7);
const $ = _c(5);
let t0;
if ($[0] !== arr1) {
t0 = [arr1];
if ($[0] !== arr1 || $[1] !== arr2) {
const x = [arr1];

let y;
t0 = () => ({ y });

(y = x.concat(arr2)), y;
$[0] = arr1;
$[1] = t0;
$[1] = arr2;
$[2] = t0;
} else {
t0 = $[1];
t0 = $[2];
}
const x = t0;
const getVal = t0;
let t1;
if ($[2] !== x || $[3] !== arr2) {
let y;
t1 = () => ({ y });

(y = x.concat(arr2)), y;
$[2] = x;
$[3] = arr2;
if ($[3] !== getVal) {
t1 = <Stringify getVal={getVal} shouldInvokeFns={true} />;
$[3] = getVal;
$[4] = t1;
} else {
t1 = $[4];
}
const getVal = t1;
let t2;
if ($[5] !== getVal) {
t2 = <Stringify getVal={getVal} shouldInvokeFns={true} />;
$[5] = getVal;
$[6] = t2;
} else {
t2 = $[6];
}
return t2;
return t1;
}

export const FIXTURE_ENTRYPOINT = {
Expand Down
43 changes: 18 additions & 25 deletions ...piler/preserve-memo-validation/useMemo-reordering-depslist-assignment.expect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,38 +30,31 @@ import { c as _c } from "react/compiler-runtime";
import { useMemo } from "react";

function useFoo(arr1, arr2) {
const $ = _c(7);
let t0;
if ($[0] !== arr1) {
t0 = [arr1];
$[0] = arr1;
$[1] = t0;
} else {
t0 = $[1];
}
const x = t0;
const $ = _c(5);
let y;
if ($[2] !== x || $[3] !== arr2) {
if ($[0] !== arr1 || $[1] !== arr2) {
const x = [arr1];

y;
(y = x.concat(arr2)), y;
$[2] = x;
$[3] = arr2;
$[4] = y;
$[0] = arr1;
$[1] = arr2;
$[2] = y;
} else {
y = $[4];
y = $[2];
}
let t1;
const t2 = y;
let t3;
if ($[5] !== t2) {
t3 = { y: t2 };
$[5] = t2;
$[6] = t3;
let t0;
const t1 = y;
let t2;
if ($[3] !== t1) {
t2 = { y: t1 };
$[3] = t1;
$[4] = t2;
} else {
t3 = $[6];
t2 = $[4];
}
t1 = t3;
return t1;
t0 = t2;
return t0;
}

export const FIXTURE_ENTRYPOINT = {
Expand Down
63 changes: 28 additions & 35 deletions ...iler/preserve-memo-validation/useMemo-reordering-depslist-controlflow.expect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,55 +40,48 @@ import { useMemo } from "react";
import { Stringify } from "shared-runtime";

function Foo(t0) {
const $ = _c(11);
const $ = _c(9);
const { arr1, arr2, foo } = t0;
let t1;
if ($[0] !== arr1) {
t1 = [arr1];
$[0] = arr1;
$[1] = t1;
} else {
t1 = $[1];
}
const x = t1;
let t2;
let val1;
if ($[2] !== foo || $[3] !== x || $[4] !== arr2) {
if ($[0] !== arr1 || $[1] !== foo || $[2] !== arr2) {
const x = [arr1];

let y;
y = [];
let t2;
let t3;
let t4;
if ($[7] === Symbol.for("react.memo_cache_sentinel")) {
t4 = { x: 2 };
$[7] = t4;
if ($[5] === Symbol.for("react.memo_cache_sentinel")) {
t3 = { x: 2 };
$[5] = t3;
} else {
t4 = $[7];
t3 = $[5];
}
t3 = t4;
val1 = t3;
t2 = t3;
val1 = t2;

foo ? (y = x.concat(arr2)) : y;
t2 = (() => [y])();
$[2] = foo;
$[3] = x;
$[4] = arr2;
$[5] = t2;
$[6] = val1;
t1 = (() => [y])();
$[0] = arr1;
$[1] = foo;
$[2] = arr2;
$[3] = t1;
$[4] = val1;
} else {
t2 = $[5];
val1 = $[6];
t1 = $[3];
val1 = $[4];
}
const val2 = t2;
let t3;
if ($[8] !== val1 || $[9] !== val2) {
t3 = <Stringify val1={val1} val2={val2} />;
$[8] = val1;
$[9] = val2;
$[10] = t3;
const val2 = t1;
let t2;
if ($[6] !== val1 || $[7] !== val2) {
t2 = <Stringify val1={val1} val2={val2} />;
$[6] = val1;
$[7] = val2;
$[8] = t2;
} else {
t3 = $[10];
t2 = $[8];
}
return t3;
return t2;
}

export const FIXTURE_ENTRYPOINT = {
Expand Down
1 change: 0 additions & 1 deletion compiler/packages/snap/src/SproutTodoFilter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,6 @@ const skipFilter = new Set([
"original-reactive-scopes-fork/bug-nonmutating-capture-in-unsplittable-memo-block",
"original-reactive-scopes-fork/bug-hoisted-declaration-with-scope",
"bug-codegen-inline-iife",
"bug-array-concat-should-capture",

// 'react-compiler-runtime' not yet supported
"flag-enable-emit-hook-guards",
Expand Down