Skip to content

Commit

Permalink
[compiler] Repro for aliased captures within inner function expressio…
Browse files Browse the repository at this point in the history
…ns (#31770)

see fixture
  • Loading branch information
mofeiZ authored Dec 16, 2024
1 parent e30872a commit 80b81fe
Show file tree
Hide file tree
Showing 3 changed files with 178 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@

## Input

```javascript
import {makeArray, mutate} from 'shared-runtime';

/**
* Bug repro:
* Found differences in evaluator results
* Non-forget (expected):
* (kind: ok)
* {"bar":4,"x":{"foo":3,"wat0":"joe"}}
* {"bar":5,"x":{"foo":3,"wat0":"joe"}}
* Forget:
* (kind: ok)
* {"bar":4,"x":{"foo":3,"wat0":"joe"}}
* {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}}
*
* Fork of `capturing-func-alias-captured-mutate`, but instead of directly
* aliasing `y` via `[y]`, we make an opaque call.
*
* Note that the bug here is that we don't infer that `a = makeArray(y)`
* potentially captures a context variable into a local variable. As a result,
* we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're
* currently inferring that this lambda captures `y` (for a potential later
* mutation) and simply reads `x`.
*
* Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not
* used when we analyze CallExpressions.
*/
function Component({foo, bar}: {foo: number; bar: number}) {
let x = {foo};
let y: {bar: number; x?: {foo: number}} = {bar};
const f0 = function () {
let a = makeArray(y); // a = [y]
let b = x;
// this writes y.x = x
a[0].x = b;
};
f0();
mutate(y.x);
return y;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{foo: 3, bar: 4}],
sequentialRenders: [
{foo: 3, bar: 4},
{foo: 3, bar: 5},
],
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime";
import { makeArray, mutate } from "shared-runtime";

/**
* Bug repro:
* Found differences in evaluator results
* Non-forget (expected):
* (kind: ok)
* {"bar":4,"x":{"foo":3,"wat0":"joe"}}
* {"bar":5,"x":{"foo":3,"wat0":"joe"}}
* Forget:
* (kind: ok)
* {"bar":4,"x":{"foo":3,"wat0":"joe"}}
* {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}}
*
* Fork of `capturing-func-alias-captured-mutate`, but instead of directly
* aliasing `y` via `[y]`, we make an opaque call.
*
* Note that the bug here is that we don't infer that `a = makeArray(y)`
* potentially captures a context variable into a local variable. As a result,
* we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're
* currently inferring that this lambda captures `y` (for a potential later
* mutation) and simply reads `x`.
*
* Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not
* used when we analyze CallExpressions.
*/
function Component(t0) {
const $ = _c(5);
const { foo, bar } = t0;
let t1;
if ($[0] !== foo) {
t1 = { foo };
$[0] = foo;
$[1] = t1;
} else {
t1 = $[1];
}
const x = t1;
let y;
if ($[2] !== bar || $[3] !== x) {
y = { bar };
const f0 = function () {
const a = makeArray(y);
const b = x;

a[0].x = b;
};

f0();
mutate(y.x);
$[2] = bar;
$[3] = x;
$[4] = y;
} else {
y = $[4];
}
return y;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ foo: 3, bar: 4 }],
sequentialRenders: [
{ foo: 3, bar: 4 },
{ foo: 3, bar: 5 },
],
};

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import {makeArray, mutate} from 'shared-runtime';

/**
* Bug repro:
* Found differences in evaluator results
* Non-forget (expected):
* (kind: ok)
* {"bar":4,"x":{"foo":3,"wat0":"joe"}}
* {"bar":5,"x":{"foo":3,"wat0":"joe"}}
* Forget:
* (kind: ok)
* {"bar":4,"x":{"foo":3,"wat0":"joe"}}
* {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}}
*
* Fork of `capturing-func-alias-captured-mutate`, but instead of directly
* aliasing `y` via `[y]`, we make an opaque call.
*
* Note that the bug here is that we don't infer that `a = makeArray(y)`
* potentially captures a context variable into a local variable. As a result,
* we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're
* currently inferring that this lambda captures `y` (for a potential later
* mutation) and simply reads `x`.
*
* Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not
* used when we analyze CallExpressions.
*/
function Component({foo, bar}: {foo: number; bar: number}) {
let x = {foo};
let y: {bar: number; x?: {foo: number}} = {bar};
const f0 = function () {
let a = makeArray(y); // a = [y]
let b = x;
// this writes y.x = x
a[0].x = b;
};
f0();
mutate(y.x);
return y;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{foo: 3, bar: 4}],
sequentialRenders: [
{foo: 3, bar: 4},
{foo: 3, bar: 5},
],
};
1 change: 1 addition & 0 deletions compiler/packages/snap/src/SproutTodoFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,7 @@ const skipFilter = new Set([
// bugs
'fbt/bug-fbt-plural-multiple-function-calls',
'fbt/bug-fbt-plural-multiple-mixed-call-tag',
`bug-capturing-func-maybealias-captured-mutate`,
'bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr',
'bug-invalid-hoisting-functionexpr',
'bug-aliased-capture-aliased-mutate',
Expand Down

0 comments on commit 80b81fe

Please sign in to comment.