Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[compiler][optim] Add shape for Array.from #32522

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
18 changes: 16 additions & 2 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,29 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [
],
/*
* https://tc39.es/ecma262/multipage/indexed-collections.html#sec-array.from
* Array.from(arrayLike, optionalFn, optionalThis) not added because
* the Effect of `arrayLike` is polymorphic i.e.
* Array.from(arrayLike, optionalFn, optionalThis)
* Note that the Effect of `arrayLike` is polymorphic i.e.
* - Effect.read if
* - it does not have an @iterator property and is array-like
* (i.e. has a length property)
* - it is an iterable object whose iterator does not mutate itself
* - Effect.mutate if it is a self-mutative iterator (e.g. a generator
* function)
*/
[
'from',
addFunction(DEFAULT_SHAPES, [], {
positionalParams: [
Effect.ConditionallyMutate,
Effect.ConditionallyMutate,
Effect.ConditionallyMutate,
],
restParam: Effect.Read,
returnType: {kind: 'Object', shapeId: BuiltInArrayId},
calleeEffect: Effect.Read,
returnValueKind: ValueKind.Mutable,
}),
],
[
'of',
// Array.of(element0, ..., elementN)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -872,11 +872,31 @@ function inferBlock(
reason: new Set([ValueReason.Other]),
context: new Set(),
};

for (const element of instrValue.elements) {
if (element.kind === 'Spread') {
state.referenceAndRecordEffects(
freezeActions,
element.place,
Effect.ConditionallyMutate,
ValueReason.Other,
);
} else if (element.kind === 'Identifier') {
state.referenceAndRecordEffects(
freezeActions,
element,
Effect.Capture,
ValueReason.Other,
);
} else {
let _: 'Hole' = element.kind;
}
}
state.initialize(instrValue, valueKind);
state.define(instr.lvalue, instrValue);
instr.lvalue.effect = Effect.Store;
continuation = {
kind: 'initialize',
valueKind,
effect: {kind: Effect.Capture, reason: ValueReason.Other},
lvalueEffect: Effect.Store,
kind: 'funeffects',
};
break;
}
Expand Down Expand Up @@ -1241,21 +1261,12 @@ function inferBlock(
for (let i = 0; i < instrValue.args.length; i++) {
const arg = instrValue.args[i];
const place = arg.kind === 'Identifier' ? arg : arg.place;
if (effects !== null) {
state.referenceAndRecordEffects(
freezeActions,
place,
effects[i],
ValueReason.Other,
);
} else {
state.referenceAndRecordEffects(
freezeActions,
place,
Effect.ConditionallyMutate,
ValueReason.Other,
);
}
state.referenceAndRecordEffects(
freezeActions,
place,
getArgumentEffect(effects != null ? effects[i] : null, arg),
ValueReason.Other,
);
hasCaptureArgument ||= place.effect === Effect.Capture;
}
if (signature !== null) {
Expand Down Expand Up @@ -1307,7 +1318,10 @@ function inferBlock(
signature !== null
? {
kind: signature.returnValueKind,
reason: new Set([ValueReason.Other]),
reason: new Set([
signature.returnValueReason ??
ValueReason.KnownReturnSignature,
]),
context: new Set(),
}
: {
Expand All @@ -1330,7 +1344,8 @@ function inferBlock(
state.referenceAndRecordEffects(
freezeActions,
place,
Effect.Read,
// see call-spread-argument-mutable-iterator test fixture
arg.kind === 'Spread' ? Effect.ConditionallyMutate : Effect.Read,
ValueReason.Other,
);
}
Expand All @@ -1356,25 +1371,16 @@ function inferBlock(
for (let i = 0; i < instrValue.args.length; i++) {
const arg = instrValue.args[i];
const place = arg.kind === 'Identifier' ? arg : arg.place;
if (effects !== null) {
/*
* If effects are inferred for an argument, we should fail invalid
* mutating effects
*/
state.referenceAndRecordEffects(
freezeActions,
place,
effects[i],
ValueReason.Other,
);
} else {
state.referenceAndRecordEffects(
freezeActions,
place,
Effect.ConditionallyMutate,
ValueReason.Other,
);
}
/*
* If effects are inferred for an argument, we should fail invalid
* mutating effects
*/
state.referenceAndRecordEffects(
freezeActions,
place,
getArgumentEffect(effects != null ? effects[i] : null, arg),
ValueReason.Other,
);
hasCaptureArgument ||= place.effect === Effect.Capture;
}
if (signature !== null) {
Expand Down Expand Up @@ -2049,3 +2055,31 @@ function areArgumentsImmutableAndNonMutating(
}
return true;
}

function getArgumentEffect(
signatureEffect: Effect | null,
arg: Place | SpreadPattern,
): Effect {
if (signatureEffect != null) {
if (arg.kind === 'Identifier') {
return signatureEffect;
} else if (
signatureEffect === Effect.Mutate ||
signatureEffect === Effect.ConditionallyMutate
) {
return signatureEffect;
} else {
// see call-spread-argument-mutable-iterator test fixture
if (signatureEffect === Effect.Freeze) {
CompilerError.throwTodo({
reason: 'Support spread syntax for hook arguments',
loc: arg.place.loc,
});
}
// effects[i] is Effect.Capture | Effect.Read | Effect.Store
return Effect.ConditionallyMutate;
}
} else {
return Effect.ConditionallyMutate;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@

## Input

```javascript
function useBar({arg}) {
/**
* Note that mutableIterator is mutated by the later object spread. Therefore,
* `s.values()` should be memoized within the same block as the object spread.
* In terms of compiler internals, they should have the same reactive scope.
*/
const obj = {};
const s = new Set([obj, 5, 4]);
const mutableIterator = s.values();
const arr = [...mutableIterator];

obj.x = arg;
return arr;
}

export const FIXTURE_ENTRYPOINT = {
fn: useBar,
params: [{arg: 3}],
sequentialRenders: [{arg: 3}, {arg: 3}, {arg: 4}],
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime";
function useBar(t0) {
const $ = _c(2);
const { arg } = t0;
let arr;
if ($[0] !== arg) {
const obj = {};
const s = new Set([obj, 5, 4]);
const mutableIterator = s.values();
arr = [...mutableIterator];

obj.x = arg;
$[0] = arg;
$[1] = arr;
} else {
arr = $[1];
}
return arr;
}

export const FIXTURE_ENTRYPOINT = {
fn: useBar,
params: [{ arg: 3 }],
sequentialRenders: [{ arg: 3 }, { arg: 3 }, { arg: 4 }],
};

```
### Eval output
(kind: ok) [{"x":3},5,4]
[{"x":3},5,4]
[{"x":4},5,4]
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
function useBar({arg}) {
/**
* Note that mutableIterator is mutated by the later object spread. Therefore,
* `s.values()` should be memoized within the same block as the object spread.
* In terms of compiler internals, they should have the same reactive scope.
*/
const obj = {};
const s = new Set([obj, 5, 4]);
const mutableIterator = s.values();
const arr = [...mutableIterator];

obj.x = arg;
return arr;
}

export const FIXTURE_ENTRYPOINT = {
fn: useBar,
params: [{arg: 3}],
sequentialRenders: [{arg: 3}, {arg: 3}, {arg: 4}],
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@

## Input

```javascript
/**
* TODO: object spreads should have conditionally mutate semantics
* Found differences in evaluator results
* Non-forget (expected):
* (kind: ok) [3,1,5,4]
* [3,1,5,4]
* [4,1,5,4]
* Forget:
* (kind: ok) [3,1,5,4]
* [3,1,5,4]
* [4]
*/

function useBar({arg}) {
'use memo';

/**
* Note that mutableIterator is mutated by the later object spread. Therefore,
* `s.values()` should be memoized within the same block as the object spread.
* In terms of compiler internals, they should have the same reactive scope.
*/
const s = new Set([1, 5, 4]);
const mutableIterator = s.values();

return [arg, ...mutableIterator];
}

export const FIXTURE_ENTRYPOINT = {
fn: useBar,
params: [{arg: 3}],
sequentialRenders: [{arg: 3}, {arg: 3}, {arg: 4}],
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime"; /**
* TODO: object spreads should have conditionally mutate semantics
* Found differences in evaluator results
* Non-forget (expected):
* (kind: ok) [3,1,5,4]
* [3,1,5,4]
* [4,1,5,4]
* Forget:
* (kind: ok) [3,1,5,4]
* [3,1,5,4]
* [4]
*/

function useBar(t0) {
"use memo";
const $ = _c(2);
const { arg } = t0;
let t1;
if ($[0] !== arg) {
const s = new Set([1, 5, 4]);
const mutableIterator = s.values();

t1 = [arg, ...mutableIterator];
$[0] = arg;
$[1] = t1;
} else {
t1 = $[1];
}
return t1;
}

export const FIXTURE_ENTRYPOINT = {
fn: useBar,
params: [{ arg: 3 }],
sequentialRenders: [{ arg: 3 }, { arg: 3 }, { arg: 4 }],
};

```

### Eval output
(kind: ok) [3,1,5,4]
[3,1,5,4]
[4,1,5,4]
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* TODO: object spreads should have conditionally mutate semantics
* Found differences in evaluator results
* Non-forget (expected):
* (kind: ok) [3,1,5,4]
* [3,1,5,4]
* [4,1,5,4]
* Forget:
* (kind: ok) [3,1,5,4]
* [3,1,5,4]
* [4]
*/

function useBar({arg}) {
'use memo';

/**
* Note that mutableIterator is mutated by the later object spread. Therefore,
* `s.values()` should be memoized within the same block as the object spread.
* In terms of compiler internals, they should have the same reactive scope.
*/
const s = new Set([1, 5, 4]);
const mutableIterator = s.values();

return [arg, ...mutableIterator];
}

export const FIXTURE_ENTRYPOINT = {
fn: useBar,
params: [{arg: 3}],
sequentialRenders: [{arg: 3}, {arg: 3}, {arg: 4}],
};
Loading
Loading