mirror of
https://github.com/facebook/react.git
synced 2025-11-01 09:12:30 +00:00
[compiler][newinference] Error handling and related fixes
Lots of small fixes related to error handling. InferMutationAliasRanges now tracks transitive calls that may mutate frozen or global values. We properly populate and track the reason each value has the kind it has, to use when throwing errors for invalid mutations (can't mutate state vs can't mutate a captured jsx value, etc). When we infer mutation effects for inner functions, we populate the location of mutations as the location where the mutation occurred, not the declaration of the captured value (aside: this was quite involved to do in the old inference, it's trivial here). A bunch of other small fixes that make sense in context.
And some of our "bug-*" fixtures output changes...becasue the new inference fixes the bugs. One example included here.
ghstack-source-id: 2c11291351
Pull Request resolved: https://github.com/facebook/react/pull/33458
This commit is contained in:
Joe Savona
@@ -266,8 +266,15 @@ function runWithEnvironment(
|
||||
inferMutableRanges(hir);
|
||||
log({kind: 'hir', name: 'InferMutableRanges', value: hir});
|
||||
} else {
|
||||
inferMutationAliasingRanges(hir);
|
||||
const mutabilityAliasingErrors = inferMutationAliasingRanges(hir, {
|
||||
isFunctionExpression: false,
|
||||
});
|
||||
log({kind: 'hir', name: 'InferMutationAliasingRanges', value: hir});
|
||||
if (env.isInferredMemoEnabled) {
|
||||
if (mutabilityAliasingErrors.isErr()) {
|
||||
throw mutabilityAliasingErrors.unwrapErr();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (env.isInferredMemoEnabled) {
|
||||
|
||||
@@ -246,7 +246,7 @@ export const EnvironmentConfigSchema = z.object({
|
||||
/**
|
||||
* Enable a new model for mutability and aliasing inference
|
||||
*/
|
||||
enableNewMutationAliasingModel: z.boolean().default(false),
|
||||
enableNewMutationAliasingModel: z.boolean().default(true),
|
||||
|
||||
/**
|
||||
* Enables inference of optional dependency chains. Without this flag
|
||||
|
||||
@@ -336,6 +336,7 @@ addObject(BUILTIN_SHAPES, BuiltInArrayId, [
|
||||
kind: 'Create',
|
||||
into: signatureArgument(2),
|
||||
value: ValueKind.Primitive,
|
||||
reason: ValueReason.KnownReturnSignature,
|
||||
},
|
||||
],
|
||||
},
|
||||
@@ -391,6 +392,7 @@ addObject(BUILTIN_SHAPES, BuiltInArrayId, [
|
||||
kind: 'Create',
|
||||
into: signatureArgument(2),
|
||||
value: ValueKind.Mutable,
|
||||
reason: ValueReason.KnownReturnSignature,
|
||||
},
|
||||
// The first arg to the callback is an item extracted from the receiver array
|
||||
{
|
||||
@@ -403,6 +405,7 @@ addObject(BUILTIN_SHAPES, BuiltInArrayId, [
|
||||
kind: 'Create',
|
||||
into: signatureArgument(5),
|
||||
value: ValueKind.Primitive,
|
||||
reason: ValueReason.KnownReturnSignature,
|
||||
},
|
||||
// calls the callback, returning the result into a temporary
|
||||
{
|
||||
|
||||
@@ -63,7 +63,7 @@ function lowerWithMutationAliasing(fn: HIRFunction): void {
|
||||
analyseFunctions(fn);
|
||||
inferMutationAliasingEffects(fn, {isFunctionExpression: true});
|
||||
deadCodeElimination(fn);
|
||||
inferMutationAliasingRanges(fn);
|
||||
inferMutationAliasingRanges(fn, {isFunctionExpression: true});
|
||||
rewriteInstructionKindsBasedOnReassignment(fn);
|
||||
inferReactiveScopeVariables(fn);
|
||||
const effects = inferMutationAliasingFunctionEffects(fn);
|
||||
|
||||
+66
-30
@@ -26,6 +26,7 @@ import {
|
||||
isArrayType,
|
||||
isMapType,
|
||||
isPrimitiveType,
|
||||
isRefOrRefValue,
|
||||
isSetType,
|
||||
makeIdentifierId,
|
||||
ObjectMethod,
|
||||
@@ -206,7 +207,6 @@ class Context {
|
||||
const hash = hashEffect(effect);
|
||||
let interned = this.internedEffects.get(hash);
|
||||
if (interned == null) {
|
||||
console.log(`intern: ${hash}`);
|
||||
this.internedEffects.set(hash, effect);
|
||||
interned = effect;
|
||||
}
|
||||
@@ -334,7 +334,6 @@ function applySignature(
|
||||
}
|
||||
const value = state.kind(effect.value);
|
||||
switch (value.kind) {
|
||||
case ValueKind.Global:
|
||||
case ValueKind.Frozen: {
|
||||
const reason = getWriteErrorReason({
|
||||
kind: value.kind,
|
||||
@@ -353,7 +352,7 @@ function applySignature(
|
||||
description:
|
||||
effect.value.identifier.name !== null &&
|
||||
effect.value.identifier.name.kind === 'named'
|
||||
? `Found mutation of \`${effect.value.identifier.name}\``
|
||||
? `Found mutation of \`${effect.value.identifier.name.value}\``
|
||||
: null,
|
||||
loc: effect.value.loc,
|
||||
suggestions: null,
|
||||
@@ -428,7 +427,7 @@ function applyEffect(
|
||||
}
|
||||
state.initialize(value, {
|
||||
kind: effect.value,
|
||||
reason: new Set([ValueReason.Other]),
|
||||
reason: new Set([effect.reason]),
|
||||
});
|
||||
state.define(effect.into, value);
|
||||
break;
|
||||
@@ -448,7 +447,7 @@ function applyEffect(
|
||||
break;
|
||||
}
|
||||
case 'CreateFrom': {
|
||||
const kind = state.kind(effect.from).kind;
|
||||
const fromValue = state.kind(effect.from);
|
||||
let value = context.effectInstructionValueCache.get(effect);
|
||||
if (value == null) {
|
||||
value = {
|
||||
@@ -459,11 +458,11 @@ function applyEffect(
|
||||
context.effectInstructionValueCache.set(effect, value);
|
||||
}
|
||||
state.initialize(value, {
|
||||
kind,
|
||||
reason: new Set([ValueReason.Other]),
|
||||
kind: fromValue.kind,
|
||||
reason: new Set(fromValue.reason),
|
||||
});
|
||||
state.define(effect.into, value);
|
||||
switch (kind) {
|
||||
switch (fromValue.kind) {
|
||||
case ValueKind.Primitive:
|
||||
case ValueKind.Global: {
|
||||
// no need to track this data flow
|
||||
@@ -587,7 +586,8 @@ function applyEffect(
|
||||
* Alias represents potential pointer aliasing. If the type is a global,
|
||||
* a primitive (copy-on-write semantics) then we can prune the effect
|
||||
*/
|
||||
const fromKind = state.kind(effect.from).kind;
|
||||
const fromValue = state.kind(effect.from);
|
||||
const fromKind = fromValue.kind;
|
||||
switch (fromKind) {
|
||||
case ValueKind.Frozen: {
|
||||
effects.push({
|
||||
@@ -604,7 +604,10 @@ function applyEffect(
|
||||
};
|
||||
context.effectInstructionValueCache.set(effect, value);
|
||||
}
|
||||
state.initialize(value, {kind: fromKind, reason: new Set([])});
|
||||
state.initialize(value, {
|
||||
kind: fromKind,
|
||||
reason: new Set(fromValue.reason),
|
||||
});
|
||||
state.define(effect.into, value);
|
||||
break;
|
||||
}
|
||||
@@ -619,7 +622,10 @@ function applyEffect(
|
||||
};
|
||||
context.effectInstructionValueCache.set(effect, value);
|
||||
}
|
||||
state.initialize(value, {kind: fromKind, reason: new Set([])});
|
||||
state.initialize(value, {
|
||||
kind: fromKind,
|
||||
reason: new Set(fromValue.reason),
|
||||
});
|
||||
state.define(effect.into, value);
|
||||
break;
|
||||
}
|
||||
@@ -722,6 +728,7 @@ function applyEffect(
|
||||
kind: 'Create',
|
||||
into: effect.into,
|
||||
value: ValueKind.Mutable,
|
||||
reason: ValueReason.Other,
|
||||
},
|
||||
aliased,
|
||||
effects,
|
||||
@@ -798,6 +805,8 @@ function applyEffect(
|
||||
const mutationKind = state.mutate(effect.kind, effect.value);
|
||||
if (mutationKind === 'mutate') {
|
||||
effects.push(effect);
|
||||
} else if (mutationKind === 'mutate-ref') {
|
||||
// no-op
|
||||
} else if (
|
||||
mutationKind !== 'none' &&
|
||||
(effect.kind === 'Mutate' || effect.kind === 'MutateTransitive')
|
||||
@@ -823,7 +832,7 @@ function applyEffect(
|
||||
description:
|
||||
effect.value.identifier.name !== null &&
|
||||
effect.value.identifier.name.kind === 'named'
|
||||
? `Found mutation of \`${effect.value.identifier.name}\``
|
||||
? `Found mutation of \`${effect.value.identifier.name.value}\``
|
||||
: null,
|
||||
loc: effect.value.loc,
|
||||
suggestions: null,
|
||||
@@ -1010,6 +1019,9 @@ class InferenceState {
|
||||
kind: ValueKind.Frozen,
|
||||
reason: new Set([reason]),
|
||||
});
|
||||
if (DEBUG) {
|
||||
console.log(`freeze value: ${printInstructionValue(value)} ${reason}`);
|
||||
}
|
||||
if (
|
||||
value.kind === 'FunctionExpression' &&
|
||||
(this.env.config.enablePreserveExistingMemoizationGuarantees ||
|
||||
@@ -1028,7 +1040,10 @@ class InferenceState {
|
||||
| 'MutateTransitive'
|
||||
| 'MutateTransitiveConditionally',
|
||||
place: Place,
|
||||
): 'none' | 'mutate' | 'mutate-frozen' | 'mutate-global' {
|
||||
): 'none' | 'mutate' | 'mutate-frozen' | 'mutate-global' | 'mutate-ref' {
|
||||
if (isRefOrRefValue(place.identifier)) {
|
||||
return 'mutate-ref';
|
||||
}
|
||||
const kind = this.kind(place).kind;
|
||||
switch (variant) {
|
||||
case 'MutateConditionally':
|
||||
@@ -1261,6 +1276,7 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
value: ValueKind.Mutable,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
// All elements are captured into part of the output value
|
||||
for (const element of value.elements) {
|
||||
@@ -1287,6 +1303,7 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
value: ValueKind.Mutable,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
for (const property of value.properties) {
|
||||
if (property.kind === 'ObjectProperty') {
|
||||
@@ -1310,6 +1327,7 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
value: ValueKind.Mutable,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
// Potentially mutates the receiver (awaiting it changes its state and can run side effects)
|
||||
effects.push({kind: 'MutateTransitiveConditionally', value: value.value});
|
||||
@@ -1366,6 +1384,7 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
value: ValueKind.Primitive,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
// Mutates the object by removing the property, no aliasing
|
||||
effects.push({kind: 'Mutate', value: value.object});
|
||||
@@ -1378,6 +1397,7 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
value: ValueKind.Primitive,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
} else {
|
||||
effects.push({
|
||||
@@ -1400,6 +1420,7 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
value: ValueKind.Primitive,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
break;
|
||||
}
|
||||
@@ -1458,6 +1479,7 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
value: ValueKind.Mutable,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
if (
|
||||
isArrayType(value.collection.identifier) ||
|
||||
@@ -1508,6 +1530,7 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
value: ValueKind.Primitive,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
break;
|
||||
}
|
||||
@@ -1517,6 +1540,7 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
value: ValueKind.Frozen,
|
||||
reason: ValueReason.JsxCaptured,
|
||||
});
|
||||
for (const operand of eachInstructionValueOperand(value)) {
|
||||
effects.push({
|
||||
@@ -1538,12 +1562,14 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: value.lvalue.place,
|
||||
value: ValueKind.Mutable,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
effects.push({
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
// The result can't be referenced so this value doesn't matter
|
||||
value: ValueKind.Primitive,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
break;
|
||||
}
|
||||
@@ -1554,12 +1580,14 @@ function computeSignatureForInstruction(
|
||||
into: value.lvalue.place,
|
||||
// TODO: what kind here???
|
||||
value: ValueKind.Primitive,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
effects.push({
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
// TODO: what kind here???
|
||||
value: ValueKind.Primitive,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
break;
|
||||
}
|
||||
@@ -1589,6 +1617,7 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: value.lvalue.place,
|
||||
value: ValueKind.Mutable,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
}
|
||||
// Which aliases the value
|
||||
@@ -1619,11 +1648,13 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
value: ValueKind.Primitive,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
effects.push({
|
||||
kind: 'Create',
|
||||
into: value.lvalue,
|
||||
value: ValueKind.Primitive,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
break;
|
||||
}
|
||||
@@ -1632,19 +1663,11 @@ function computeSignatureForInstruction(
|
||||
kind: 'MutateGlobal',
|
||||
place: value.value,
|
||||
error: {
|
||||
severity: ErrorSeverity.InvalidReact,
|
||||
reason: getWriteErrorReason({
|
||||
kind: ValueKind.Global,
|
||||
reason: new Set([ValueReason.Global]),
|
||||
context: new Set(),
|
||||
}),
|
||||
description:
|
||||
value.value.identifier.name !== null &&
|
||||
value.value.identifier.name.kind === 'named'
|
||||
? `Found mutation of \`${value.value.identifier.name}\``
|
||||
: null,
|
||||
loc: value.value.loc,
|
||||
reason:
|
||||
'Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render)',
|
||||
loc: instr.loc,
|
||||
suggestions: null,
|
||||
severity: ErrorSeverity.InvalidReact,
|
||||
},
|
||||
});
|
||||
effects.push({kind: 'Assign', from: value.value, into: lvalue});
|
||||
@@ -1659,6 +1682,7 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
value: ValueKind.Global,
|
||||
reason: ValueReason.Global,
|
||||
});
|
||||
break;
|
||||
}
|
||||
@@ -1677,6 +1701,7 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
value: ValueKind.Primitive,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
break;
|
||||
}
|
||||
@@ -1694,6 +1719,7 @@ function computeSignatureForInstruction(
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
value: ValueKind.Primitive,
|
||||
reason: ValueReason.Other,
|
||||
});
|
||||
break;
|
||||
}
|
||||
@@ -1719,16 +1745,16 @@ function computeEffectsForLegacySignature(
|
||||
receiver: Place,
|
||||
args: Array<Place | SpreadPattern | Hole>,
|
||||
): Array<AliasingEffect> {
|
||||
const returnValueReason = signature.returnValueReason ?? ValueReason.Other;
|
||||
const effects: Array<AliasingEffect> = [];
|
||||
effects.push({
|
||||
kind: 'Create',
|
||||
into: lvalue,
|
||||
value: signature.returnValueKind,
|
||||
reason: returnValueReason,
|
||||
});
|
||||
const stores: Array<Place> = [];
|
||||
const captures: Array<Place> = [];
|
||||
const returnValueReason =
|
||||
signature.returnValueReason ?? ValueReason.KnownReturnSignature;
|
||||
function visit(place: Place, effect: Effect): void {
|
||||
switch (effect) {
|
||||
case Effect.Store: {
|
||||
@@ -2046,7 +2072,12 @@ function computeEffectsForSignature(
|
||||
case 'Create': {
|
||||
const into = substitutions.get(effect.into.identifier.id) ?? [];
|
||||
for (const value of into) {
|
||||
effects.push({kind: 'Create', into: value, value: effect.value});
|
||||
effects.push({
|
||||
kind: 'Create',
|
||||
into: value,
|
||||
value: effect.value,
|
||||
reason: effect.reason,
|
||||
});
|
||||
}
|
||||
break;
|
||||
}
|
||||
@@ -2297,7 +2328,7 @@ export type AliasingEffect =
|
||||
/**
|
||||
* Creates a value of the given type at the given place
|
||||
*/
|
||||
| {kind: 'Create'; into: Place; value: ValueKind}
|
||||
| {kind: 'Create'; into: Place; value: ValueKind; reason: ValueReason}
|
||||
/**
|
||||
* Creates a new value with the same kind as the starting value.
|
||||
*/
|
||||
@@ -2376,7 +2407,12 @@ function hashEffect(effect: AliasingEffect): string {
|
||||
].join(':');
|
||||
}
|
||||
case 'Create': {
|
||||
return [effect.kind, effect.into.identifier.id].join(':');
|
||||
return [
|
||||
effect.kind,
|
||||
effect.into.identifier.id,
|
||||
effect.value,
|
||||
effect.reason,
|
||||
].join(':');
|
||||
}
|
||||
case 'Freeze': {
|
||||
return [effect.kind, effect.value.identifier.id, effect.reason].join(':');
|
||||
|
||||
+2
-1
@@ -5,7 +5,7 @@
|
||||
* LICENSE file in the root directory of this source tree.
|
||||
*/
|
||||
|
||||
import {HIRFunction, IdentifierId, Place, ValueKind} from '../HIR';
|
||||
import {HIRFunction, IdentifierId, Place, ValueKind, ValueReason} from '../HIR';
|
||||
import {getOrInsertDefault} from '../Utils/utils';
|
||||
import {AliasingEffect} from './InferMutationAliasingEffects';
|
||||
|
||||
@@ -157,6 +157,7 @@ export function inferMutationAliasingFunctionEffects(
|
||||
fn.returnType.kind === 'Primitive'
|
||||
? ValueKind.Primitive
|
||||
: ValueKind.Mutable,
|
||||
reason: ValueReason.KnownReturnSignature,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
+112
-55
@@ -6,7 +6,7 @@
|
||||
*/
|
||||
|
||||
import prettyFormat from 'pretty-format';
|
||||
import {CompilerError} from '..';
|
||||
import {CompilerError, SourceLocation} from '..';
|
||||
import {
|
||||
BlockId,
|
||||
Effect,
|
||||
@@ -26,6 +26,7 @@ import {assertExhaustive, getOrInsertWith} from '../Utils/utils';
|
||||
import {printFunction} from '../HIR';
|
||||
import {printIdentifier, printPlace} from '../HIR/PrintHIR';
|
||||
import {MutationKind} from './InferMutationAliasingFunctionEffects';
|
||||
import {Result} from '../Utils/Result';
|
||||
|
||||
const DEBUG = false;
|
||||
const VERBOSE = false;
|
||||
@@ -33,18 +34,11 @@ const VERBOSE = false;
|
||||
/**
|
||||
* Infers mutable ranges for all values.
|
||||
*/
|
||||
export function inferMutationAliasingRanges(fn: HIRFunction): void {
|
||||
export function inferMutationAliasingRanges(
|
||||
fn: HIRFunction,
|
||||
{isFunctionExpression}: {isFunctionExpression: boolean},
|
||||
): Result<void, CompilerError> {
|
||||
if (VERBOSE) {
|
||||
console.log();
|
||||
console.log();
|
||||
console.log();
|
||||
console.log();
|
||||
console.log();
|
||||
console.log();
|
||||
console.log();
|
||||
console.log();
|
||||
console.log();
|
||||
console.log();
|
||||
console.log();
|
||||
console.log(printFunction(fn));
|
||||
}
|
||||
@@ -75,14 +69,16 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
|
||||
|
||||
let index = 0;
|
||||
|
||||
const errors = new CompilerError();
|
||||
|
||||
for (const param of [...fn.params, ...fn.context, fn.returns]) {
|
||||
const place = param.kind === 'Identifier' ? param : param.place;
|
||||
state.create(place);
|
||||
state.create(place, {kind: 'Object'});
|
||||
}
|
||||
const seenBlocks = new Set<BlockId>();
|
||||
for (const block of fn.body.blocks.values()) {
|
||||
for (const phi of block.phis) {
|
||||
state.create(phi.place, true);
|
||||
state.create(phi.place, {kind: 'Phi'});
|
||||
for (const [pred, operand] of phi.operands) {
|
||||
if (!seenBlocks.has(pred)) {
|
||||
// NOTE: annotation required to actually typecheck and not silently infer `any`
|
||||
@@ -100,14 +96,29 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
|
||||
seenBlocks.add(block.id);
|
||||
|
||||
for (const instr of block.instructions) {
|
||||
for (const lvalue of eachInstructionLValue(instr)) {
|
||||
state.create(lvalue);
|
||||
if (
|
||||
instr.value.kind === 'FunctionExpression' ||
|
||||
instr.value.kind === 'ObjectMethod'
|
||||
) {
|
||||
state.create(instr.lvalue, {
|
||||
kind: 'Function',
|
||||
function: instr.value.loweredFunc.func,
|
||||
});
|
||||
} else {
|
||||
for (const lvalue of eachInstructionLValue(instr)) {
|
||||
state.create(lvalue, {kind: 'Object'});
|
||||
}
|
||||
}
|
||||
|
||||
if (instr.effects == null) continue;
|
||||
for (const effect of instr.effects) {
|
||||
if (effect.kind === 'Create' || effect.kind === 'CreateFunction') {
|
||||
state.create(effect.into);
|
||||
if (effect.kind === 'Create') {
|
||||
state.create(effect.into, {kind: 'Object'});
|
||||
} else if (effect.kind === 'CreateFunction') {
|
||||
state.create(effect.into, {
|
||||
kind: 'Function',
|
||||
function: effect.function.loweredFunc.func,
|
||||
});
|
||||
} else if (effect.kind === 'CreateFrom') {
|
||||
state.createFrom(index++, effect.from, effect.into);
|
||||
} else if (effect.kind === 'Assign' || effect.kind === 'Alias') {
|
||||
@@ -142,6 +153,11 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
|
||||
: MutationKind.Conditional,
|
||||
place: effect.value,
|
||||
});
|
||||
} else if (
|
||||
effect.kind === 'MutateFrozen' ||
|
||||
effect.kind === 'MutateGlobal'
|
||||
) {
|
||||
errors.push(effect.error);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -184,6 +200,8 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
|
||||
makeInstructionId(mutation.id + 1),
|
||||
mutation.transitive,
|
||||
mutation.kind,
|
||||
mutation.place.loc,
|
||||
errors,
|
||||
);
|
||||
}
|
||||
if (VERBOSE) {
|
||||
@@ -197,31 +215,35 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
|
||||
continue;
|
||||
}
|
||||
let mutated = false;
|
||||
if (node.local === MutationKind.Conditional) {
|
||||
mutated = true;
|
||||
fn.aliasingEffects.push({
|
||||
kind: 'MutateConditionally',
|
||||
value: place,
|
||||
});
|
||||
} else if (node.local === MutationKind.Definite) {
|
||||
mutated = true;
|
||||
fn.aliasingEffects.push({
|
||||
kind: 'Mutate',
|
||||
value: place,
|
||||
});
|
||||
if (node.local != null) {
|
||||
if (node.local.kind === MutationKind.Conditional) {
|
||||
mutated = true;
|
||||
fn.aliasingEffects.push({
|
||||
kind: 'MutateConditionally',
|
||||
value: {...place, loc: node.local.loc},
|
||||
});
|
||||
} else if (node.local.kind === MutationKind.Definite) {
|
||||
mutated = true;
|
||||
fn.aliasingEffects.push({
|
||||
kind: 'Mutate',
|
||||
value: {...place, loc: node.local.loc},
|
||||
});
|
||||
}
|
||||
}
|
||||
if (node.transitive === MutationKind.Conditional) {
|
||||
mutated = true;
|
||||
fn.aliasingEffects.push({
|
||||
kind: 'MutateTransitiveConditionally',
|
||||
value: place,
|
||||
});
|
||||
} else if (node.transitive === MutationKind.Definite) {
|
||||
mutated = true;
|
||||
fn.aliasingEffects.push({
|
||||
kind: 'MutateTransitive',
|
||||
value: place,
|
||||
});
|
||||
if (node.transitive != null) {
|
||||
if (node.transitive.kind === MutationKind.Conditional) {
|
||||
mutated = true;
|
||||
fn.aliasingEffects.push({
|
||||
kind: 'MutateTransitiveConditionally',
|
||||
value: {...place, loc: node.transitive.loc},
|
||||
});
|
||||
} else if (node.transitive.kind === MutationKind.Definite) {
|
||||
mutated = true;
|
||||
fn.aliasingEffects.push({
|
||||
kind: 'MutateTransitive',
|
||||
value: {...place, loc: node.transitive.loc},
|
||||
});
|
||||
}
|
||||
}
|
||||
if (mutated) {
|
||||
place.effect = Effect.Capture;
|
||||
@@ -360,14 +382,33 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
|
||||
operand.effect = effect;
|
||||
}
|
||||
}
|
||||
for (const operand of eachTerminalOperand(block.terminal)) {
|
||||
operand.effect = Effect.Read;
|
||||
if (block.terminal.kind === 'return') {
|
||||
block.terminal.value.effect = isFunctionExpression
|
||||
? Effect.Read
|
||||
: Effect.Freeze;
|
||||
} else {
|
||||
for (const operand of eachTerminalOperand(block.terminal)) {
|
||||
operand.effect = Effect.Read;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (VERBOSE) {
|
||||
console.log(printFunction(fn));
|
||||
}
|
||||
return errors.asResult();
|
||||
}
|
||||
|
||||
function appendFunctionErrors(errors: CompilerError, fn: HIRFunction): void {
|
||||
for (const effect of fn.aliasingEffects ?? []) {
|
||||
switch (effect.kind) {
|
||||
case 'MutateFrozen':
|
||||
case 'MutateGlobal': {
|
||||
errors.push(effect.error);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
type Node = {
|
||||
@@ -376,28 +417,31 @@ type Node = {
|
||||
captures: Map<Identifier, number>;
|
||||
aliases: Map<Identifier, number>;
|
||||
edges: Array<{index: number; node: Identifier; kind: 'capture' | 'alias'}>;
|
||||
transitive: MutationKind;
|
||||
local: MutationKind;
|
||||
phi: boolean;
|
||||
transitive: {kind: MutationKind; loc: SourceLocation} | null;
|
||||
local: {kind: MutationKind; loc: SourceLocation} | null;
|
||||
value:
|
||||
| {kind: 'Object'}
|
||||
| {kind: 'Phi'}
|
||||
| {kind: 'Function'; function: HIRFunction};
|
||||
};
|
||||
class AliasingState {
|
||||
nodes: Map<Identifier, Node> = new Map();
|
||||
|
||||
create(place: Place, phi: boolean = false): void {
|
||||
create(place: Place, value: Node['value']): void {
|
||||
this.nodes.set(place.identifier, {
|
||||
id: place.identifier,
|
||||
createdFrom: new Map(),
|
||||
captures: new Map(),
|
||||
aliases: new Map(),
|
||||
edges: [],
|
||||
transitive: MutationKind.None,
|
||||
local: MutationKind.None,
|
||||
phi,
|
||||
transitive: null,
|
||||
local: null,
|
||||
value,
|
||||
});
|
||||
}
|
||||
|
||||
createFrom(index: number, from: Place, into: Place): void {
|
||||
this.create(into);
|
||||
this.create(into, {kind: 'Object'});
|
||||
const fromNode = this.nodes.get(from.identifier);
|
||||
const toNode = this.nodes.get(into.identifier);
|
||||
if (fromNode == null || toNode == null) {
|
||||
@@ -454,6 +498,8 @@ class AliasingState {
|
||||
end: InstructionId,
|
||||
transitive: boolean,
|
||||
kind: MutationKind,
|
||||
loc: SourceLocation,
|
||||
errors: CompilerError,
|
||||
): void {
|
||||
const seen = new Set<Identifier>();
|
||||
const queue: Array<{
|
||||
@@ -484,10 +530,21 @@ class AliasingState {
|
||||
node.id.mutableRange.end = makeInstructionId(
|
||||
Math.max(node.id.mutableRange.end, end),
|
||||
);
|
||||
if (
|
||||
node.value.kind === 'Function' &&
|
||||
node.transitive == null &&
|
||||
node.local == null
|
||||
) {
|
||||
appendFunctionErrors(errors, node.value.function);
|
||||
}
|
||||
if (transitive) {
|
||||
node.transitive = Math.max(node.transitive, kind);
|
||||
if (node.transitive == null || node.transitive.kind < kind) {
|
||||
node.transitive = {kind, loc};
|
||||
}
|
||||
} else {
|
||||
node.local = Math.max(node.local, kind);
|
||||
if (node.local == null || node.local.kind < kind) {
|
||||
node.local = {kind, loc};
|
||||
}
|
||||
}
|
||||
/**
|
||||
* all mutations affect "forward" edges by the rules:
|
||||
@@ -506,7 +563,7 @@ class AliasingState {
|
||||
}
|
||||
queue.push({place: alias, transitive: true, direction: 'backwards'});
|
||||
}
|
||||
if (direction === 'backwards' || !node.phi) {
|
||||
if (direction === 'backwards' || node.value.kind !== 'Phi') {
|
||||
/**
|
||||
* all mutations affect backward alias edges by the rules:
|
||||
* - Alias a -> b, mutate(b) => mutate(a)
|
||||
|
||||
+25
@@ -112,6 +112,31 @@ export function validateNoFreezingKnownMutableFunctions(
|
||||
);
|
||||
if (knownMutation && knownMutation.kind === 'ContextMutation') {
|
||||
contextMutationEffects.set(lvalue.identifier.id, knownMutation);
|
||||
} else if (
|
||||
fn.env.config.enableNewMutationAliasingModel &&
|
||||
value.loweredFunc.func.aliasingEffects != null
|
||||
) {
|
||||
const context = new Set(
|
||||
value.loweredFunc.func.context.map(p => p.identifier.id),
|
||||
);
|
||||
effects: for (const effect of value.loweredFunc.func
|
||||
.aliasingEffects) {
|
||||
switch (effect.kind) {
|
||||
case 'Mutate':
|
||||
case 'MutateTransitive': {
|
||||
if (context.has(effect.value.identifier.id)) {
|
||||
contextMutationEffects.set(lvalue.identifier.id, {
|
||||
kind: 'ContextMutation',
|
||||
effect: Effect.Mutate,
|
||||
loc: effect.value.loc,
|
||||
places: new Set([effect.value]),
|
||||
});
|
||||
break effects;
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
+125
@@ -0,0 +1,125 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @enableNewMutationAliasingModel
|
||||
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"; // @enableNewMutationAliasingModel
|
||||
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(3);
|
||||
const { foo, bar } = t0;
|
||||
let y;
|
||||
if ($[0] !== bar || $[1] !== foo) {
|
||||
const x = { foo };
|
||||
y = { bar };
|
||||
const f0 = function () {
|
||||
const a = makeArray(y);
|
||||
const b = x;
|
||||
|
||||
a[0].x = b;
|
||||
};
|
||||
|
||||
f0();
|
||||
mutate(y.x);
|
||||
$[0] = bar;
|
||||
$[1] = foo;
|
||||
$[2] = y;
|
||||
} else {
|
||||
y = $[2];
|
||||
}
|
||||
return y;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: Component,
|
||||
params: [{ foo: 3, bar: 4 }],
|
||||
sequentialRenders: [
|
||||
{ foo: 3, bar: 4 },
|
||||
{ foo: 3, bar: 5 },
|
||||
],
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
### Eval output
|
||||
(kind: ok) {"bar":4,"x":{"foo":3,"wat0":"joe"}}
|
||||
{"bar":5,"x":{"foo":3,"wat0":"joe"}}
|
||||
+49
@@ -0,0 +1,49 @@
|
||||
// @enableNewMutationAliasingModel
|
||||
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},
|
||||
],
|
||||
};
|
||||
Reference in New Issue
Block a user