Skip to content

Commit

Permalink
Update on "[compiler] More complete validation against locals being r…
Browse files Browse the repository at this point in the history
…eassigned after render"

Summary:
This diff extends the existing work on validating against locals being reassigned after render, by propagating the reassignment "effect" into the lvalues of instructions when the rvalue operands include values known to cause reassignments. In particular, this "closes the loop" for function definitions and function calls: a function that returns a function that reassigns will be considered to also perform reassignments, but previous to this we didn't consider the result of a `Call` of a function that reassigns to itself be a value that reassigns.

This causes a number of new bailouts in test cases, all of which appear to me to be legit.

[ghstack-poisoned]
  • Loading branch information
mvitousek committed Jul 31, 2024
1 parent b24b6ea commit f011de8
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
eachInstructionValueOperand,
eachTerminalOperand,
} from '../HIR/visitors';
import {getFunctionCallSignature} from '../Inference/InferReferenceEffects';

/**
* Validates that local variables cannot be reassigned after render.
Expand Down Expand Up @@ -132,7 +133,26 @@ function getContextReassignment(
break;
}
default: {
for (const operand of eachInstructionValueOperand(value)) {
let operands = eachInstructionValueOperand(value);
// If we're calling a function that doesn't let its arguments escape, only test the callee
if (value.kind === 'CallExpression') {
const signature = getFunctionCallSignature(
fn.env,
value.callee.identifier.type,
);
if (signature?.noAlias) {
operands = [value.callee];
}
} else if (value.kind === 'MethodCall') {
const signature = getFunctionCallSignature(
fn.env,
value.property.identifier.type,
);
if (signature?.noAlias) {
operands = [value.receiver, value.property];
}
}
for (const operand of operands) {
CompilerError.invariant(operand.effect !== Effect.Unknown, {
reason: `Expected effects to be inferred prior to ValidateLocalsNotReassignedAfterRender`,
loc: operand.loc,
Expand All @@ -148,6 +168,10 @@ function getContextReassignment(
if (operand.effect === Effect.Freeze) {
return reassignment;
} else {
/*
* If the operand is not frozen but it does reassign, then the lvalues
* of the instruction could also be reassigning
*/
for (const lval of eachInstructionLValue(instr)) {
reassigningFunctions.set(lval.identifier.id, reassignment);
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@

## Input

```javascript
import {useEffect, useState} from 'react';
import {mutate} from 'shared-runtime';

function Component(props) {
const x = [{...props.value}];
useEffect(() => {}, []);
const onClick = () => {
console.log(x.length);
};
let y;
return (
<div onClick={onClick}>
{x.map(item => {
y = item;
return <span key={item.id}>{item.text}</span>;
})}
{mutate(y)}
</div>
);
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{value: {id: 0, text: 'Hello!'}}],
isComponent: true,
};

```

## Code

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

function Component(props) {
const $ = _c(5);
const x = [{ ...props.value }];
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = [];
$[0] = t0;
} else {
t0 = $[0];
}
useEffect(_temp, t0);
const onClick = () => {
console.log(x.length);
};

let y;

const t1 = x.map((item) => {
y = item;
return <span key={item.id}>{item.text}</span>;
});
const t2 = mutate(y);
let t3;
if ($[1] !== onClick || $[2] !== t1 || $[3] !== t2) {
t3 = (
<div onClick={onClick}>
{t1}
{t2}
</div>
);
$[1] = onClick;
$[2] = t1;
$[3] = t2;
$[4] = t3;
} else {
t3 = $[4];
}
return t3;
}
function _temp() {}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ value: { id: 0, text: "Hello!" } }],
isComponent: true,
};

```
### Eval output
(kind: ok) <div><span>Hello!</span></div>

0 comments on commit f011de8

Please sign in to comment.