mirror of
https://github.com/facebook/react.git
synced 2025-11-01 09:12:30 +00:00
[compiler] Fix codegen for nested method calls with memoized properties
When processing nested method calls like `Math.floor(diff.bar())`, the compiler would trigger an invariant that `MethodCall::property must be a MemberExpression but got an Identifier`. The issue occurred when the property (e.g., Math.floor) was memoized in a reactive scope and promoted to a named identifier. Later during codegen, retrieving this memoized temporary would return just an Identifier instead of the expected MemberExpression. The fix handles this case by checking if the property has been memoized as an Identifier and using it directly for the call expression, rather than requiring it to be a MemberExpression. This fixes two test cases that were previously failing: - error.bug-invariant-codegen-methodcall - error.todo-nested-method-calls-lower-property-load-into-temporary
This commit is contained in:
Lauren Tan
+64
-35
@@ -1810,41 +1810,70 @@ function codegenInstructionValue(
|
||||
case 'MethodCall': {
|
||||
const isHook =
|
||||
getHookKind(cx.env, instrValue.property.identifier) != null;
|
||||
const memberExpr = codegenPlaceToExpression(cx, instrValue.property);
|
||||
CompilerError.invariant(
|
||||
t.isMemberExpression(memberExpr) ||
|
||||
t.isOptionalMemberExpression(memberExpr),
|
||||
{
|
||||
reason:
|
||||
'[Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. ' +
|
||||
`Got a \`${memberExpr.type}\``,
|
||||
description: null,
|
||||
loc: memberExpr.loc ?? null,
|
||||
suggestions: null,
|
||||
},
|
||||
);
|
||||
CompilerError.invariant(
|
||||
t.isNodesEquivalent(
|
||||
memberExpr.object,
|
||||
codegenPlaceToExpression(cx, instrValue.receiver),
|
||||
),
|
||||
{
|
||||
reason:
|
||||
'[Codegen] Internal error: Forget should always generate MethodCall::property ' +
|
||||
'as a MemberExpression of MethodCall::receiver',
|
||||
description: null,
|
||||
loc: memberExpr.loc ?? null,
|
||||
suggestions: null,
|
||||
},
|
||||
);
|
||||
const args = instrValue.args.map(arg => codegenArgument(cx, arg));
|
||||
value = createCallExpression(
|
||||
cx.env,
|
||||
memberExpr,
|
||||
args,
|
||||
instrValue.loc,
|
||||
isHook,
|
||||
);
|
||||
/**
|
||||
* We need to check if the property was memoized. If it has, we should reconstruct the
|
||||
* MemberExpression.
|
||||
**/
|
||||
let memberExpr: t.Expression;
|
||||
const tmp = cx.temp.get(instrValue.property.identifier.declarationId);
|
||||
if (tmp != null && tmp.type === 'Identifier') {
|
||||
/**
|
||||
* We can't reconstruct the MemberExpression from just the identifier, so we work around
|
||||
* this by allowing an Identifier here.
|
||||
*/
|
||||
memberExpr = tmp;
|
||||
} else if (tmp != null) {
|
||||
memberExpr = convertValueToExpression(tmp);
|
||||
} else {
|
||||
memberExpr = codegenPlaceToExpression(cx, instrValue.property);
|
||||
}
|
||||
|
||||
// Reconstruct the MemberExpression if we previously saw an Identifier.
|
||||
if (memberExpr.type === 'Identifier') {
|
||||
const args = instrValue.args.map(arg => codegenArgument(cx, arg));
|
||||
value = createCallExpression(
|
||||
cx.env,
|
||||
memberExpr,
|
||||
args,
|
||||
instrValue.loc,
|
||||
isHook,
|
||||
);
|
||||
} else {
|
||||
CompilerError.invariant(
|
||||
t.isMemberExpression(memberExpr) ||
|
||||
t.isOptionalMemberExpression(memberExpr),
|
||||
{
|
||||
reason:
|
||||
'[Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. ' +
|
||||
`Got a \`${memberExpr.type}\``,
|
||||
description: null,
|
||||
loc: memberExpr.loc ?? null,
|
||||
suggestions: null,
|
||||
},
|
||||
);
|
||||
CompilerError.invariant(
|
||||
t.isNodesEquivalent(
|
||||
memberExpr.object,
|
||||
codegenPlaceToExpression(cx, instrValue.receiver),
|
||||
),
|
||||
{
|
||||
reason:
|
||||
'[Codegen] Internal error: Forget should always generate MethodCall::property ' +
|
||||
'as a MemberExpression of MethodCall::receiver',
|
||||
description: null,
|
||||
loc: memberExpr.loc ?? null,
|
||||
suggestions: null,
|
||||
},
|
||||
);
|
||||
const args = instrValue.args.map(arg => codegenArgument(cx, arg));
|
||||
value = createCallExpression(
|
||||
cx.env,
|
||||
memberExpr,
|
||||
args,
|
||||
instrValue.loc,
|
||||
isHook,
|
||||
);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case 'NewExpression': {
|
||||
|
||||
+49
@@ -0,0 +1,49 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
const YearsAndMonthsSince = () => {
|
||||
const diff = foo();
|
||||
const months = Math.floor(diff.bar());
|
||||
return <>{months}</>;
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
## Code
|
||||
|
||||
```javascript
|
||||
import { c as _c } from "react/compiler-runtime";
|
||||
const YearsAndMonthsSince = () => {
|
||||
const $ = _c(4);
|
||||
let t0;
|
||||
let t1;
|
||||
let t2;
|
||||
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
const diff = foo();
|
||||
t0 = Math;
|
||||
t1 = t0.floor;
|
||||
t2 = diff.bar();
|
||||
$[0] = t0;
|
||||
$[1] = t1;
|
||||
$[2] = t2;
|
||||
} else {
|
||||
t0 = $[0];
|
||||
t1 = $[1];
|
||||
t2 = $[2];
|
||||
}
|
||||
const months = t1(t2);
|
||||
let t3;
|
||||
if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
t3 = <>{months}</>;
|
||||
$[3] = t3;
|
||||
} else {
|
||||
t3 = $[3];
|
||||
}
|
||||
return t3;
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
### Eval output
|
||||
(kind: exception) Fixture not implemented
|
||||
-31
@@ -1,31 +0,0 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
const YearsAndMonthsSince = () => {
|
||||
const diff = foo();
|
||||
const months = Math.floor(diff.bar());
|
||||
return <>{months}</>;
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
|
||||
## Error
|
||||
|
||||
```
|
||||
Found 1 error:
|
||||
|
||||
Invariant: [Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. Got a `Identifier`
|
||||
|
||||
error.bug-invariant-codegen-methodcall.ts:3:17
|
||||
1 | const YearsAndMonthsSince = () => {
|
||||
2 | const diff = foo();
|
||||
> 3 | const months = Math.floor(diff.bar());
|
||||
| ^^^^^^^^^^ [Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. Got a `Identifier`
|
||||
4 | return <>{months}</>;
|
||||
5 | };
|
||||
6 |
|
||||
```
|
||||
|
||||
|
||||
-39
@@ -1,39 +0,0 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
import {makeArray} from 'shared-runtime';
|
||||
|
||||
const other = [0, 1];
|
||||
function Component({}) {
|
||||
const items = makeArray(0, 1, 2, null, 4, false, 6);
|
||||
const max = Math.max(2, items.push(5), ...other);
|
||||
return max;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: Component,
|
||||
params: [{}],
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
|
||||
## Error
|
||||
|
||||
```
|
||||
Found 1 error:
|
||||
|
||||
Invariant: [Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. Got a `Identifier`
|
||||
|
||||
error.todo-nested-method-calls-lower-property-load-into-temporary.ts:6:14
|
||||
4 | function Component({}) {
|
||||
5 | const items = makeArray(0, 1, 2, null, 4, false, 6);
|
||||
> 6 | const max = Math.max(2, items.push(5), ...other);
|
||||
| ^^^^^^^^ [Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. Got a `Identifier`
|
||||
7 | return max;
|
||||
8 | }
|
||||
9 |
|
||||
```
|
||||
|
||||
|
||||
+62
@@ -0,0 +1,62 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
import {makeArray} from 'shared-runtime';
|
||||
|
||||
const other = [0, 1];
|
||||
function Component({}) {
|
||||
const items = makeArray(0, 1, 2, null, 4, false, 6);
|
||||
const max = Math.max(2, items.push(5), ...other);
|
||||
return max;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: Component,
|
||||
params: [{}],
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
## Code
|
||||
|
||||
```javascript
|
||||
import { c as _c } from "react/compiler-runtime";
|
||||
import { makeArray } from "shared-runtime";
|
||||
|
||||
const other = [0, 1];
|
||||
function Component(t0) {
|
||||
const $ = _c(4);
|
||||
let t1;
|
||||
let t2;
|
||||
let t3;
|
||||
let t4;
|
||||
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
|
||||
const items = makeArray(0, 1, 2, null, 4, false, 6);
|
||||
t1 = Math;
|
||||
t2 = t1.max;
|
||||
t3 = 2;
|
||||
t4 = items.push(5);
|
||||
$[0] = t1;
|
||||
$[1] = t2;
|
||||
$[2] = t3;
|
||||
$[3] = t4;
|
||||
} else {
|
||||
t1 = $[0];
|
||||
t2 = $[1];
|
||||
t3 = $[2];
|
||||
t4 = $[3];
|
||||
}
|
||||
const max = t2(t3, t4, ...other);
|
||||
return max;
|
||||
}
|
||||
|
||||
export const FIXTURE_ENTRYPOINT = {
|
||||
fn: Component,
|
||||
params: [{}],
|
||||
};
|
||||
|
||||
```
|
||||
|
||||
### Eval output
|
||||
(kind: ok) 8
|
||||
Reference in New Issue
Block a user