mirror of
https://github.com/zebrajr/react.git
synced 2025-12-06 12:20:20 +01:00
[compiler] Fix AnalyzeFunctions to fully reset context identifiers (#33500)
AnalyzeFunctions had logic to reset the mutable ranges of context variables after visiting inner function expressions. However, there was a bug in that logic: InferReactiveScopeVariables makes all the identifiers in a scope point to the same mutable range instance. That meant that it was possible for a later function expression to indirectly cause an earlier function expressions' context variables to get a non-zero mutable range. The fix is to not just reset start/end of context var ranges, but assign a new range instance. Thanks for the help on debugging, @mofeiz! --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33500). * #33571 * #33558 * #33547 * #33543 * #33533 * #33532 * #33530 * #33526 * #33522 * #33518 * #33514 * #33513 * #33512 * #33504 * __->__ #33500 * #33497 * #33496
This commit is contained in:
parent
90ccbd71c1
commit
7c28c15465
|
|
@ -42,8 +42,16 @@ export default function analyseFunctions(func: HIRFunction): void {
|
||||||
* Reset mutable range for outer inferReferenceEffects
|
* Reset mutable range for outer inferReferenceEffects
|
||||||
*/
|
*/
|
||||||
for (const operand of instr.value.loweredFunc.func.context) {
|
for (const operand of instr.value.loweredFunc.func.context) {
|
||||||
operand.identifier.mutableRange.start = makeInstructionId(0);
|
/**
|
||||||
operand.identifier.mutableRange.end = makeInstructionId(0);
|
* NOTE: inferReactiveScopeVariables makes identifiers in the scope
|
||||||
|
* point to the *same* mutableRange instance. Resetting start/end
|
||||||
|
* here is insufficient, because a later mutation of the range
|
||||||
|
* for any one identifier could affect the range for other identifiers.
|
||||||
|
*/
|
||||||
|
operand.identifier.mutableRange = {
|
||||||
|
start: makeInstructionId(0),
|
||||||
|
end: makeInstructionId(0),
|
||||||
|
};
|
||||||
operand.identifier.scope = null;
|
operand.identifier.scope = null;
|
||||||
}
|
}
|
||||||
break;
|
break;
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,80 @@
|
||||||
|
|
||||||
|
## Input
|
||||||
|
|
||||||
|
```javascript
|
||||||
|
//@flow @validatePreserveExistingMemoizationGuarantees @enableNewMutationAliasingModel
|
||||||
|
component Component(
|
||||||
|
onAsyncSubmit?: (() => void) => void,
|
||||||
|
onClose: (isConfirmed: boolean) => void
|
||||||
|
) {
|
||||||
|
// When running inferReactiveScopeVariables,
|
||||||
|
// onAsyncSubmit and onClose update to share
|
||||||
|
// a mutableRange instance.
|
||||||
|
const onSubmit = useCallback(() => {
|
||||||
|
if (onAsyncSubmit) {
|
||||||
|
onAsyncSubmit(() => {
|
||||||
|
onClose(true);
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}, [onAsyncSubmit, onClose]);
|
||||||
|
// When running inferReactiveScopeVariables here,
|
||||||
|
// first the existing range gets updated (affecting
|
||||||
|
// onAsyncSubmit) and then onClose gets assigned a
|
||||||
|
// different mutable range instance, which is the
|
||||||
|
// one reset after AnalyzeFunctions.
|
||||||
|
// The fix is to fully reset mutable ranges *instances*
|
||||||
|
// after AnalyzeFunctions visit a function expression
|
||||||
|
return <Dialog onSubmit={onSubmit} onClose={() => onClose(false)} />;
|
||||||
|
}
|
||||||
|
|
||||||
|
```
|
||||||
|
|
||||||
|
## Code
|
||||||
|
|
||||||
|
```javascript
|
||||||
|
import { c as _c } from "react/compiler-runtime";
|
||||||
|
function Component(t0) {
|
||||||
|
const $ = _c(8);
|
||||||
|
const { onAsyncSubmit, onClose } = t0;
|
||||||
|
let t1;
|
||||||
|
if ($[0] !== onAsyncSubmit || $[1] !== onClose) {
|
||||||
|
t1 = () => {
|
||||||
|
if (onAsyncSubmit) {
|
||||||
|
onAsyncSubmit(() => {
|
||||||
|
onClose(true);
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
$[0] = onAsyncSubmit;
|
||||||
|
$[1] = onClose;
|
||||||
|
$[2] = t1;
|
||||||
|
} else {
|
||||||
|
t1 = $[2];
|
||||||
|
}
|
||||||
|
const onSubmit = t1;
|
||||||
|
let t2;
|
||||||
|
if ($[3] !== onClose) {
|
||||||
|
t2 = () => onClose(false);
|
||||||
|
$[3] = onClose;
|
||||||
|
$[4] = t2;
|
||||||
|
} else {
|
||||||
|
t2 = $[4];
|
||||||
|
}
|
||||||
|
let t3;
|
||||||
|
if ($[5] !== onSubmit || $[6] !== t2) {
|
||||||
|
t3 = <Dialog onSubmit={onSubmit} onClose={t2} />;
|
||||||
|
$[5] = onSubmit;
|
||||||
|
$[6] = t2;
|
||||||
|
$[7] = t3;
|
||||||
|
} else {
|
||||||
|
t3 = $[7];
|
||||||
|
}
|
||||||
|
return t3;
|
||||||
|
}
|
||||||
|
|
||||||
|
```
|
||||||
|
|
||||||
|
### Eval output
|
||||||
|
(kind: exception) Fixture not implemented
|
||||||
|
|
@ -0,0 +1,25 @@
|
||||||
|
//@flow @validatePreserveExistingMemoizationGuarantees @enableNewMutationAliasingModel
|
||||||
|
component Component(
|
||||||
|
onAsyncSubmit?: (() => void) => void,
|
||||||
|
onClose: (isConfirmed: boolean) => void
|
||||||
|
) {
|
||||||
|
// When running inferReactiveScopeVariables,
|
||||||
|
// onAsyncSubmit and onClose update to share
|
||||||
|
// a mutableRange instance.
|
||||||
|
const onSubmit = useCallback(() => {
|
||||||
|
if (onAsyncSubmit) {
|
||||||
|
onAsyncSubmit(() => {
|
||||||
|
onClose(true);
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}, [onAsyncSubmit, onClose]);
|
||||||
|
// When running inferReactiveScopeVariables here,
|
||||||
|
// first the existing range gets updated (affecting
|
||||||
|
// onAsyncSubmit) and then onClose gets assigned a
|
||||||
|
// different mutable range instance, which is the
|
||||||
|
// one reset after AnalyzeFunctions.
|
||||||
|
// The fix is to fully reset mutable ranges *instances*
|
||||||
|
// after AnalyzeFunctions visit a function expression
|
||||||
|
return <Dialog onSubmit={onSubmit} onClose={() => onClose(false)} />;
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue
Block a user