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.

Sign up for GitHub

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

Jump to bottom

[compiler] Type inference for tagged template literals #30869

Merged
merged 2 commits into from
Sep 4, 2024
Merged

[compiler] Type inference for tagged template literals #30869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bea8495
[compiler] Type inference for tagged template literals
josephsavona Sep 4, 2024
fbf79e0
Update on "[compiler] Type inference for tagged template literals"
josephsavona Sep 4, 2024
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
53 changes: 41 additions & 12 deletions compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1180,18 +1180,6 @@ function inferBlock(
};
break;
}
case 'TaggedTemplateExpression': {
valueKind = {
kind: ValueKind.Mutable,
reason: new Set([ValueReason.Other]),
context: new Set(),
};
effect = {
kind: Effect.ConditionallyMutate,
reason: ValueReason.Other,
};
break;
}
case 'TemplateLiteral': {
/*
* template literal (with no tag function) always produces
Expand Down Expand Up @@ -1312,6 +1300,47 @@ function inferBlock(
instr.lvalue.effect = Effect.Store;
continue;
}
case 'TaggedTemplateExpression': {
const operands = [...eachInstructionValueOperand(instrValue)];
if (operands.length !== 1) {
// future-proofing to make sure we update this case when we support interpolation
CompilerError.throwTodo({
reason: 'Support tagged template expressions with interpolations',
loc: instrValue.loc,
});
}
const signature = getFunctionCallSignature(
env,
instrValue.tag.identifier.type,
);
let calleeEffect =
signature?.calleeEffect ?? Effect.ConditionallyMutate;
const returnValueKind: AbstractValue =
signature !== null
? {
kind: signature.returnValueKind,
reason: new Set([
signature.returnValueReason ??
ValueReason.KnownReturnSignature,
]),
context: new Set(),
}
: {
kind: ValueKind.Mutable,
reason: new Set([ValueReason.Other]),
context: new Set(),
};
state.referenceAndRecordEffects(
instrValue.tag,
calleeEffect,
ValueReason.Other,
functionEffects,
);
state.initialize(instrValue, returnValueKind);
state.define(instr.lvalue, instrValue);
instr.lvalue.effect = Effect.ConditionallyMutate;
continue;
}
case 'CallExpression': {
const signature = getFunctionCallSignature(
env,
Expand Down
4 changes: 2 additions & 2 deletions ...er/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ function mayAllocate(env: Environment, instruction: Instruction): boolean {
case 'StoreGlobal': {
return false;
}
case 'TaggedTemplateExpression':
case 'CallExpression':
case 'MethodCall': {
return instruction.lvalue.identifier.type.kind !== 'Primitive';
Expand All @@ -241,8 +242,7 @@ function mayAllocate(env: Environment, instruction: Instruction): boolean {
case 'ObjectExpression':
case 'UnsupportedNode':
case 'ObjectMethod':
case 'FunctionExpression':
case 'TaggedTemplateExpression': {
case 'FunctionExpression': {
return true;
}
default: {
Expand Down
31 changes: 28 additions & 3 deletions compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,12 +671,37 @@ function computeMemoizationInputs(
],
};
}
case 'TaggedTemplateExpression': {
const signature = getFunctionCallSignature(
env,
value.tag.identifier.type,
);
let lvalues = [];
if (lvalue !== null) {
lvalues.push({place: lvalue, level: MemoizationLevel.Memoized});
}
if (signature?.noAlias === true) {
return {
lvalues,
rvalues: [],
};
}
const operands = [...eachReactiveValueOperand(value)];
lvalues.push(
...operands
.filter(operand => isMutableEffect(operand.effect, operand.loc))
.map(place => ({place, level: MemoizationLevel.Memoized})),
);
return {
lvalues,
rvalues: operands,
};
}
case 'CallExpression': {
const signature = getFunctionCallSignature(
env,
value.callee.identifier.type,
);
const operands = [...eachReactiveValueOperand(value)];
let lvalues = [];
if (lvalue !== null) {
lvalues.push({place: lvalue, level: MemoizationLevel.Memoized});
Expand All @@ -687,6 +712,7 @@ function computeMemoizationInputs(
rvalues: [],
};
}
const operands = [...eachReactiveValueOperand(value)];
lvalues.push(
...operands
.filter(operand => isMutableEffect(operand.effect, operand.loc))
Expand All @@ -702,7 +728,6 @@ function computeMemoizationInputs(
env,
value.property.identifier.type,
);
const operands = [...eachReactiveValueOperand(value)];
let lvalues = [];
if (lvalue !== null) {
lvalues.push({place: lvalue, level: MemoizationLevel.Memoized});
Expand All @@ -713,6 +738,7 @@ function computeMemoizationInputs(
rvalues: [],
};
}
const operands = [...eachReactiveValueOperand(value)];
lvalues.push(
...operands
.filter(operand => isMutableEffect(operand.effect, operand.loc))
Expand All @@ -726,7 +752,6 @@ function computeMemoizationInputs(
case 'RegExpLiteral':
case 'ObjectMethod':
case 'FunctionExpression':
case 'TaggedTemplateExpression':
case 'ArrayExpression':
case 'NewExpression':
case 'ObjectExpression':
Expand Down
21 changes: 19 additions & 2 deletions compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ function* generateInstructionTypes(
}

case 'CallExpression': {
const returnType = makeType();
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I don't quite understand -- what does creating an intermediate returnType do here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't really matter in practice, but this ensures we have a place to put the inferred return type of the function (eg from its signature) separately from what we infer about how it's lvalue is used. For example in foo() && ..., we would inver the lvalue of the foo() call is a primitive bc of the logical expression, but that means the actual call expression's return type information is lost. This way we can still track the actual function return type.

/*
* TODO: callee could be a hook or a function, so this type equation isn't correct.
* We should change Hook to a subtype of Function or change unifier logic.
Expand All @@ -258,8 +259,25 @@ function* generateInstructionTypes(
yield equation(value.callee.identifier.type, {
kind: 'Function',
shapeId: null,
return: left,
return: returnType,
});
yield equation(left, returnType);
break;
}

case 'TaggedTemplateExpression': {
const returnType = makeType();
/*
* TODO: callee could be a hook or a function, so this type equation isn't correct.
* We should change Hook to a subtype of Function or change unifier logic.
* (see https://github.com/facebook/react-forget/pull/1427)
*/
yield equation(value.tag.identifier.type, {
kind: 'Function',
shapeId: null,
return: returnType,
});
yield equation(left, returnType);
break;
}

Expand Down Expand Up @@ -392,7 +410,6 @@ function* generateInstructionTypes(
case 'MetaProperty':
case 'ComputedStore':
case 'ComputedLoad':
case 'TaggedTemplateExpression':
case 'Await':
case 'GetIterator':
case 'IteratorNext':
Expand Down
8 changes: 8 additions & 0 deletions ...ages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,14 @@ function getContextReassignment(
if (signature?.noAlias) {
operands = [value.receiver, value.property];
}
} else if (value.kind === 'TaggedTemplateExpression') {
const signature = getFunctionCallSignature(
fn.env,
value.tag.identifier.type,
);
if (signature?.noAlias) {
operands = [value.tag];
}
}
for (const operand of operands) {
CompilerError.invariant(operand.effect !== Effect.Unknown, {
Expand Down
74 changes: 35 additions & 39 deletions ...s/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,67 +63,63 @@ function useFragment(_arg1, _arg2) {
}

function Component(props) {
const $ = _c(9);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = graphql`
const $ = _c(8);
const post = useFragment(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this fixture has a locally defined useFragment so it isn't using the type definition for the useFragment in shared-runtime.

graphql`
fragment F on T {
id
}
`;
$[0] = t0;
} else {
t0 = $[0];
}
const post = useFragment(t0, props.post);
let t1;
if ($[1] !== post) {
`,
props.post,
);
let t0;
if ($[0] !== post) {
const allUrls = [];

const { media: t2, comments: t3, urls: t4 } = post;
const media = t2 === undefined ? null : t2;
const { media: t1, comments: t2, urls: t3 } = post;
const media = t1 === undefined ? null : t1;
let t4;
if ($[2] !== t2) {
t4 = t2 === undefined ? [] : t2;
$[2] = t2;
$[3] = t4;
} else {
t4 = $[3];
}
const comments = t4;
let t5;
if ($[3] !== t3) {
if ($[4] !== t3) {
t5 = t3 === undefined ? [] : t3;
$[3] = t3;
$[4] = t5;
$[4] = t3;
$[5] = t5;
} else {
t5 = $[4];
t5 = $[5];
}
const comments = t5;
const urls = t5;
let t6;
if ($[5] !== t4) {
t6 = t4 === undefined ? [] : t4;
$[5] = t4;
$[6] = t6;
} else {
t6 = $[6];
}
const urls = t6;
let t7;
if ($[7] !== comments.length) {
t7 = (e) => {
if ($[6] !== comments.length) {
t6 = (e) => {
if (!comments.length) {
return;
}

console.log(comments.length);
};
$[7] = comments.length;
$[8] = t7;
$[6] = comments.length;
$[7] = t6;
} else {
t7 = $[8];
t6 = $[7];
}
const onClick = t7;
const onClick = t6;

allUrls.push(...urls);
t1 = <Stringify media={media} allUrls={allUrls} onClick={onClick} />;
$[1] = post;
$[2] = t1;
t0 = <Stringify media={media} allUrls={allUrls} onClick={onClick} />;
$[0] = post;
$[1] = t0;
} else {
t1 = $[2];
t0 = $[1];
}
return t1;
return t0;
}

export const FIXTURE_ENTRYPOINT = {
Expand Down
Loading