Skip to content

Commit 9d655f9

Browse files
committed
[compiler] Bail out and log calls that likely have side effects
ghstack-source-id: 3581548 Pull Request resolved: #30572
1 parent 06d0b89 commit 9d655f9

File tree

2 files changed

+130
-0
lines changed

2 files changed

+130
-0
lines changed

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ import {
9898
} from '../Validation';
9999
import {validateLocalsNotReassignedAfterRender} from '../Validation/ValidateLocalsNotReassignedAfterRender';
100100
import {outlineFunctions} from '../Optimization/OutlineFunctions';
101+
import {validateNoMutationCalls} from '../ReactiveScopes/ValidateNoMutationCalls';
101102

102103
export type CompilerPipelineValue =
103104
| {kind: 'ast'; name: string; value: CodegenFunction}
@@ -484,6 +485,8 @@ function* runWithEnvironment(
484485
validatePreservedManualMemoization(reactiveFunction);
485486
}
486487

488+
validateNoMutationCalls(reactiveFunction);
489+
487490
const ast = codegenFunction(reactiveFunction, {
488491
uniqueIdentifiers,
489492
fbtOperands,
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
import {CompilerError, ErrorSeverity} from '../CompilerError';
9+
import {Environment} from '../HIR';
10+
import {
11+
Effect,
12+
Identifier,
13+
IdentifierId,
14+
ReactiveFunction,
15+
ReactiveInstruction,
16+
getHookKind,
17+
isSetStateType,
18+
} from '../HIR/HIR';
19+
import {
20+
ReactiveFunctionVisitor,
21+
eachReactiveValueOperand,
22+
visitReactiveFunction,
23+
} from './visitors';
24+
25+
const allowedNames = new Set(['invariant']);
26+
27+
class Visitor extends ReactiveFunctionVisitor<CompilerError> {
28+
#env: Environment;
29+
#names: Map<IdentifierId, string> = new Map();
30+
31+
constructor(env: Environment) {
32+
super();
33+
this.#env = env;
34+
}
35+
36+
getName(id: Identifier): string | undefined {
37+
if (id.name?.kind === 'named') {
38+
return id.name.value;
39+
} else {
40+
return this.#names.get(id.id);
41+
}
42+
}
43+
44+
setName(id: Identifier, name: string | undefined): void {
45+
if (name != null) {
46+
this.#names.set(id.id, name);
47+
}
48+
}
49+
50+
override visitInstruction(
51+
instr: ReactiveInstruction,
52+
state: CompilerError,
53+
): void {
54+
if (instr.lvalue != null) {
55+
switch (instr.value.kind) {
56+
case 'LoadGlobal':
57+
this.setName(instr.lvalue.identifier, instr.value.binding.name);
58+
break;
59+
case 'LoadLocal':
60+
case 'LoadContext':
61+
this.setName(
62+
instr.lvalue.identifier,
63+
this.getName(instr.value.place.identifier),
64+
);
65+
break;
66+
case 'PropertyLoad': {
67+
const name = this.getName(instr.value.object.identifier);
68+
if (name != null) {
69+
this.setName(
70+
instr.lvalue.identifier,
71+
name + '.' + instr.value.property,
72+
);
73+
}
74+
break;
75+
}
76+
case 'ComputedLoad': {
77+
const name = this.getName(instr.value.object.identifier);
78+
if (name != null) {
79+
this.setName(instr.lvalue.identifier, name + '[...]');
80+
}
81+
break;
82+
}
83+
}
84+
}
85+
86+
if (
87+
instr.value.kind === 'CallExpression' ||
88+
instr.value.kind === 'MethodCall'
89+
) {
90+
let isException = false;
91+
let name = '(unknown)';
92+
if (instr.value.kind === 'CallExpression') {
93+
isException =
94+
getHookKind(this.#env, instr.value.callee.identifier) != null ||
95+
isSetStateType(instr.value.callee.identifier);
96+
name = this.getName(instr.value.callee.identifier) ?? name;
97+
} else {
98+
isException =
99+
getHookKind(this.#env, instr.value.property.identifier) != null ||
100+
isSetStateType(instr.value.property.identifier);
101+
name = this.getName(instr.value.property.identifier) ?? name;
102+
}
103+
if (instr.lvalue === null && !isException && !allowedNames.has(name)) {
104+
let allReads = true;
105+
for (const operand of eachReactiveValueOperand(instr.value)) {
106+
allReads &&= operand.effect === Effect.Read;
107+
}
108+
if (allReads) {
109+
state.push({
110+
reason: `Likely illegal mutation call \`${name}\``,
111+
loc: instr.loc,
112+
severity: ErrorSeverity.Todo,
113+
});
114+
}
115+
}
116+
}
117+
super.visitInstruction(instr, state);
118+
}
119+
}
120+
121+
export function validateNoMutationCalls(fn: ReactiveFunction): void {
122+
const error = new CompilerError();
123+
visitReactiveFunction(fn, new Visitor(fn.env), error);
124+
if (error.hasErrors()) {
125+
throw error;
126+
}
127+
}

0 commit comments

Comments
 (0)