Skip to content

Commit

Permalink
[compiler] Repro for invalid Array.map type
Browse files Browse the repository at this point in the history
See test fixture
  • Loading branch information
mofeiZ committed Jan 16, 2025
1 parent 3298434 commit 91b2834
Show file tree
Hide file tree
Showing 3 changed files with 220 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@

## Input

```javascript
import {
arrayPush,
identity,
makeArray,
Stringify,
useFragment,
} from 'shared-runtime';

/**
* Bug repro showing why it's invalid for function references to be annotated
* with a `Read` effect when that reference might lead to the function being
* invoked.
*
* Note that currently, `Array.map` is annotated to have `Read` effects on its
* operands. This is incorrect as function effects must be replayed when `map`
* is called
* - Read: non-aliasing data dependency
* - Capture: maybe-aliasing data dependency
* - ConditionallyMutate: maybe-aliasing data dependency; maybe-write / invoke
* but only if the value is mutable
*
* Invalid evaluator result: Found differences in evaluator results Non-forget
* (expected): (kind: ok)
* <div>{"x":[2,2,2],"count":3}</div><div>{"item":1}</div>
* <div>{"x":[2,2,2],"count":4}</div><div>{"item":1}</div>
* Forget:
* (kind: ok)
* <div>{"x":[2,2,2],"count":3}</div><div>{"item":1}</div>
* <div>{"x":[2,2,2,2,2,2],"count":4}</div><div>{"item":1}</div>
*/

function Component({extraJsx}) {
const x = makeArray();
const items = useFragment();
const jsx = items.a.map((item, i) => {
arrayPush(x, 2);
return <Stringify item={item} key={i} />;
});
const offset = jsx.length;
for (let i = 0; i < extraJsx; i++) {
jsx.push(<Stringify item={0} key={i + offset} />);
}
const count = jsx.length;
identity(count);
return (
<>
<Stringify x={x} count={count} />
{jsx[0]}
</>
);
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{extraJsx: 0}],
sequentialRenders: [{extraJsx: 0}, {extraJsx: 1}],
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime";
import {
arrayPush,
identity,
makeArray,
Stringify,
useFragment,
} from "shared-runtime";

/**
* Bug repro showing why it's invalid for function references to be annotated
* with a `Read` effect when that reference might lead to the function being
* invoked.
*
* Note that currently, `Array.map` is annotated to have `Read` effects on its
* operands. This is incorrect as function effects must be replayed when `map`
* is called
* - Read: non-aliasing data dependency
* - Capture: maybe-aliasing data dependency
* - ConditionallyMutate: maybe-aliasing data dependency; maybe-write / invoke
* but only if the value is mutable
*
* Invalid evaluator result: Found differences in evaluator results Non-forget
* (expected): (kind: ok)
* <div>{"x":[2,2,2],"count":3}</div><div>{"item":1}</div>
* <div>{"x":[2,2,2],"count":4}</div><div>{"item":1}</div>
* Forget:
* (kind: ok)
* <div>{"x":[2,2,2],"count":3}</div><div>{"item":1}</div>
* <div>{"x":[2,2,2,2,2,2],"count":4}</div><div>{"item":1}</div>
*/

function Component(t0) {
const $ = _c(9);
const { extraJsx } = t0;
let t1;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t1 = makeArray();
$[0] = t1;
} else {
t1 = $[0];
}
const x = t1;
const items = useFragment();
let jsx;
if ($[1] !== extraJsx || $[2] !== items.a) {
jsx = items.a.map((item, i) => {
arrayPush(x, 2);
return <Stringify item={item} key={i} />;
});
const offset = jsx.length;
for (let i_0 = 0; i_0 < extraJsx; i_0++) {
jsx.push(<Stringify item={0} key={i_0 + offset} />);
}
$[1] = extraJsx;
$[2] = items.a;
$[3] = jsx;
} else {
jsx = $[3];
}

const count = jsx.length;
identity(count);
let t2;
if ($[4] !== count) {
t2 = <Stringify x={x} count={count} />;
$[4] = count;
$[5] = t2;
} else {
t2 = $[5];
}
const t3 = jsx[0];
let t4;
if ($[6] !== t2 || $[7] !== t3) {
t4 = (
<>
{t2}
{t3}
</>
);
$[6] = t2;
$[7] = t3;
$[8] = t4;
} else {
t4 = $[8];
}
return t4;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ extraJsx: 0 }],
sequentialRenders: [{ extraJsx: 0 }, { extraJsx: 1 }],
};

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import {
arrayPush,
identity,
makeArray,
Stringify,
useFragment,
} from 'shared-runtime';

/**
* Bug repro showing why it's invalid for function references to be annotated
* with a `Read` effect when that reference might lead to the function being
* invoked.
*
* Note that currently, `Array.map` is annotated to have `Read` effects on its
* operands. This is incorrect as function effects must be replayed when `map`
* is called
* - Read: non-aliasing data dependency
* - Capture: maybe-aliasing data dependency
* - ConditionallyMutate: maybe-aliasing data dependency; maybe-write / invoke
* but only if the value is mutable
*
* Invalid evaluator result: Found differences in evaluator results Non-forget
* (expected): (kind: ok)
* <div>{"x":[2,2,2],"count":3}</div><div>{"item":1}</div>
* <div>{"x":[2,2,2],"count":4}</div><div>{"item":1}</div>
* Forget:
* (kind: ok)
* <div>{"x":[2,2,2],"count":3}</div><div>{"item":1}</div>
* <div>{"x":[2,2,2,2,2,2],"count":4}</div><div>{"item":1}</div>
*/

function Component({extraJsx}) {
const x = makeArray();
const items = useFragment();
const jsx = items.a.map((item, i) => {
arrayPush(x, 2);
return <Stringify item={item} key={i} />;
});
const offset = jsx.length;
for (let i = 0; i < extraJsx; i++) {
jsx.push(<Stringify item={0} key={i + offset} />);
}
const count = jsx.length;
identity(count);
return (
<>
<Stringify x={x} count={count} />
{jsx[0]}
</>
);
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{extraJsx: 0}],
sequentialRenders: [{extraJsx: 0}, {extraJsx: 1}],
};
1 change: 1 addition & 0 deletions compiler/packages/snap/src/SproutTodoFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ const skipFilter = new Set([
'bug-aliased-capture-mutate',
'bug-functiondecl-hoisting',
'bug-try-catch-maybe-null-dependency',
'bug-invalid-mixedreadonly-map-shape',
'bug-type-inference-control-flow',
'reduce-reactive-deps/bug-infer-function-cond-access-not-hoisted',
'bug-invalid-phi-as-dependency',
Expand Down

0 comments on commit 91b2834

Please sign in to comment.