Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,6 @@ function runWithEnvironment(
}

if (!env.config.enableNewMutationAliasingModel) {
// NOTE: in the new model this is part of validateNoFreezingKnownMutableFunctions
validateLocalsNotReassignedAfterRender(hir);
}

Expand Down Expand Up @@ -275,6 +274,7 @@ function runWithEnvironment(
if (mutabilityAliasingErrors.isErr()) {
throw mutabilityAliasingErrors.unwrapErr();
}
validateLocalsNotReassignedAfterRender(hir);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
} from '../HIR/visitors';
import {Ok, Result} from '../Utils/Result';
import {
getArgumentEffect,
getFunctionCallSignature,
isKnownMutableEffect,
mergeValueKinds,
Expand Down Expand Up @@ -547,7 +548,11 @@ function applyEffect(
effect =>
effect.kind === 'MutateFrozen' || effect.kind === 'MutateGlobal',
);
const isMutable = hasCaptures || hasTrackedSideEffects;
// For legacy compatibility
const capturesRef = effect.function.loweredFunc.func.context.some(
operand => isRefOrRefValue(operand.identifier),
);
const isMutable = hasCaptures || hasTrackedSideEffects || capturesRef;
for (const operand of effect.function.loweredFunc.func.context) {
if (operand.effect !== Effect.Capture) {
continue;
Expand Down Expand Up @@ -805,6 +810,11 @@ function applyEffect(
effects,
);
}
const mutateIterator =
arg.kind === 'Spread' ? conditionallyMutateIterator(operand) : null;
if (mutateIterator) {
applyEffect(context, state, mutateIterator, aliased, effects);
}
applyEffect(
context,
state,
Expand Down Expand Up @@ -1299,6 +1309,22 @@ type InstructionSignature = {
effects: ReadonlyArray<AliasingEffect>;
};

function conditionallyMutateIterator(place: Place): AliasingEffect | null {
if (
!(
isArrayType(place.identifier) ||
isSetType(place.identifier) ||
isMapType(place.identifier)
)
) {
return {
kind: 'MutateTransitiveConditionally',
value: place,
};
}
return null;
}

/**
* Computes an effect signature for the instruction _without_ looking at the inference state,
* and only using the semantics of the instructions and the inferred types. The idea is to make
Expand Down Expand Up @@ -1334,6 +1360,10 @@ function computeSignatureForInstruction(
into: lvalue,
});
} else if (element.kind === 'Spread') {
const mutateIterator = conditionallyMutateIterator(element.place);
if (mutateIterator != null) {
effects.push(mutateIterator);
}
effects.push({
kind: 'Capture',
from: element.place,
Expand Down Expand Up @@ -1567,7 +1597,7 @@ function computeSignatureForInstruction(
// Extracts part of the original collection into the result
effects.push({
kind: 'CreateFrom',
from: value.iterator,
from: value.collection,
into: lvalue,
});
break;
Expand Down Expand Up @@ -1623,11 +1653,20 @@ function computeSignatureForInstruction(
}
case 'Destructure': {
for (const patternLValue of eachInstructionValueLValue(value)) {
effects.push({
kind: 'CreateFrom',
from: value.value,
into: patternLValue,
});
if (isPrimitiveType(patternLValue.identifier)) {
effects.push({
kind: 'Create',
into: patternLValue,
value: ValueKind.Primitive,
reason: ValueReason.Other,
});
} else {
effects.push({
kind: 'CreateFrom',
from: value.value,
into: patternLValue,
});
}
}
effects.push({kind: 'Assign', from: value.value, into: lvalue});
break;
Expand Down Expand Up @@ -1947,17 +1986,11 @@ function computeEffectsForLegacySignature(
continue;
}
const place = arg.kind === 'Identifier' ? arg : arg.place;
const effect =
const signatureEffect =
arg.kind === 'Identifier' && i < signature.positionalParams.length
? signature.positionalParams[i]!
: (signature.restParam ?? Effect.ConditionallyMutate);

if (arg.kind === 'Spread' && effect === Effect.Freeze) {
CompilerError.throwTodo({
reason: 'Support spread syntax for hook arguments',
loc: arg.place.loc,
});
}
const effect = getArgumentEffect(signatureEffect, arg);

visit(place, effect);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,12 @@ export function inferMutationAliasingRanges(
});
} else if (effect.kind === 'CreateFrom') {
state.createFrom(index++, effect.from, effect.into);
} else if (effect.kind === 'Assign' || effect.kind === 'Alias') {
} else if (effect.kind === 'Assign') {
if (!state.nodes.has(effect.into.identifier)) {
state.create(effect.into, {kind: 'Object'});
}
state.assign(index++, effect.from, effect.into);
} else if (effect.kind === 'Alias') {
state.assign(index++, effect.from, effect.into);
} else if (effect.kind === 'Capture') {
state.capture(index++, effect.from, effect.into);
Expand Down Expand Up @@ -204,8 +209,8 @@ export function inferMutationAliasingRanges(
errors,
);
}
if (VERBOSE) {
console.log(state.debug());
if (DEBUG) {
console.log(pretty([...state.nodes.keys()]));
}
fn.aliasingEffects ??= [];
for (const param of [...fn.context, ...fn.params]) {
Expand Down Expand Up @@ -534,6 +539,11 @@ class AliasingState {
loc: SourceLocation,
errors: CompilerError,
): void {
if (DEBUG) {
console.log(
`mutate ix=${index} start=$${start.id} end=[${end}]${transitive ? ' transitive' : ''} kind=${kind}`,
);
}
const seen = new Set<Identifier>();
const queue: Array<{
place: Identifier;
Expand All @@ -557,7 +567,7 @@ class AliasingState {
}
if (DEBUG) {
console.log(
`[${end}] mutate index=${index} ${printIdentifier(start)}: ${printIdentifier(node.id)}`,
` mutate $${node.id.id} transitive=${transitive} direction=${direction}`,
);
}
node.id.mutableRange.end = makeInstructionId(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1984,7 +1984,7 @@ function areArgumentsImmutableAndNonMutating(
return true;
}

function getArgumentEffect(
export function getArgumentEffect(
signatureEffect: Effect | null,
arg: Place | SpreadPattern,
): Effect {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,10 @@ export function validateNoFreezingKnownMutableFunctions(
lvalue.identifier.id,
knownMutation,
);
} else if (context.has(effect.value.identifier.id)) {
} else if (
context.has(effect.value.identifier.id) &&
!isRefOrRefLikeMutableType(effect.value.identifier.type)
) {
contextMutationEffects.set(lvalue.identifier.id, {
kind: 'ContextMutation',
effect: Effect.Mutate,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,21 +175,14 @@ import {
* and mutability.
*/
function Component(t0) {
const $ = _c(4);
const $ = _c(2);
const { prop } = t0;
let t1;
if ($[0] !== prop) {
const obj = shallowCopy(prop);
const aliasedObj = identity(obj);
let t2;
if ($[2] !== obj) {
t2 = [obj.id];
$[2] = obj;
$[3] = t2;
} else {
t2 = $[3];
}
const id = t2;

const id = [obj.id];

mutate(aliasedObj);
setPropertyByKey(aliasedObj, "id", prop.id + 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,34 @@ export const FIXTURE_ENTRYPOINT = {
```javascript
import { c as _c } from "react/compiler-runtime";
function Component(props) {
const $ = _c(2);
const $ = _c(6);
let t0;
if ($[0] !== props.a) {
const item = { a: props.a };
const items = [item];
t0 = items.map(_temp);
t0 = { a: props.a };
$[0] = props.a;
$[1] = t0;
} else {
t0 = $[1];
}
const mapped = t0;
const item = t0;
let t1;
if ($[2] !== item) {
t1 = [item];
$[2] = item;
$[3] = t1;
} else {
t1 = $[3];
}
const items = t1;
let t2;
if ($[4] !== items) {
t2 = items.map(_temp);
$[4] = items;
$[5] = t2;
} else {
t2 = $[5];
}
const mapped = t2;
return mapped;
}
function _temp(item_0) {
Expand Down
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,11 @@ import { makeArray, mutate } from "shared-runtime";
* used when we analyze CallExpressions.
*/
function Component(t0) {
const $ = _c(5);
const $ = _c(3);
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) {
if ($[0] !== bar || $[1] !== foo) {
const x = { foo };
y = { bar };
const f0 = function () {
const a = makeArray(y);
Expand All @@ -107,11 +99,11 @@ function Component(t0) {

f0();
mutate(y.x);
$[2] = bar;
$[3] = x;
$[4] = y;
$[0] = bar;
$[1] = foo;
$[2] = y;
} else {
y = $[4];
y = $[2];
}
return y;
}
Expand Down
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ export const FIXTURE_ENTRYPOINT = {
## Code

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

/**
Expand All @@ -54,31 +53,14 @@ import { CONST_TRUE, Stringify, mutate, useIdentity } from "shared-runtime";
*
*/
function Component() {
const $ = _c(4);
const obj = CONST_TRUE ? { inner: { value: "hello" } } : null;
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = [obj?.inner];
$[0] = t0;
} else {
t0 = $[0];
}
const boxedInner = t0;
const boxedInner = [obj?.inner];
useIdentity(null);
mutate(obj);
if (boxedInner[0] !== obj?.inner) {
throw new Error("invariant broken");
}
let t1;
if ($[1] !== boxedInner || $[2] !== obj) {
t1 = <Stringify obj={obj} inner={boxedInner} />;
$[1] = boxedInner;
$[2] = obj;
$[3] = t1;
} else {
t1 = $[3];
}
return t1;
return <Stringify obj={obj} inner={boxedInner} />;
}

export const FIXTURE_ENTRYPOINT = {
Expand Down
Loading
Loading