[compiler] rfc: Include location information in identifiers and reactive scopes for debugging
Summary: Using the change detection code to debug codebases that violate the rules of react is a lot easier when we have a source location corresponding to the value that has changed inappropriately. I didn't see an easy way to track that information in the existing data structures at the point of codegen, so this PR adds locations to identifiers and reactive scopes (the location of a reactive scope is the range of the locations of its included identifiers). I'm interested if there's a better way to do this that I missed! ghstack-source-id: aed5f7eddae7256f41da4389e8f16fcb3daaee49 Pull Request resolved: https://github.com/facebook/react/pull/29658
Mike Vitousek committed
May 31, 2024 at 14:06 UTC
ec6fe57a5027d60a959493a2e44b6872b8de0ab8
14 files changed
+72
-29
compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
+6
-4
@@ -124,7 +124,7 @@ export function lower(
124
) {
125
const place: Place = {
126
kind: "Identifier",
127
- identifier: builder.makeTemporary(),
127
+ identifier: builder.makeTemporary(param.node.loc ?? GeneratedSource),
128
effect: Effect.Unknown,
129
reactive: false,
130
loc: param.node.loc ?? GeneratedSource,
@@ -141,7 +141,7 @@ export function lower(
141
} else if (param.isRestElement()) {
142
const place: Place = {
143
kind: "Identifier",
144
- identifier: builder.makeTemporary(),
144
+ identifier: builder.makeTemporary(param.node.loc ?? GeneratedSource),
145
effect: Effect.Unknown,
146
reactive: false,
147
loc: param.node.loc ?? GeneratedSource,
@@ -1256,7 +1256,9 @@ function lowerStatement(
1256
if (hasNode(handlerBindingPath)) {
1257
const place: Place = {
1258
kind: "Identifier",
1259
- identifier: builder.makeTemporary(),
1259
+ identifier: builder.makeTemporary(
1260
+ handlerBindingPath.node.loc ?? GeneratedSource
1261
+ ),
1262
effect: Effect.Unknown,
1263
reactive: false,
1264
loc: handlerBindingPath.node.loc ?? GeneratedSource,
@@ -3301,7 +3303,7 @@ function lowerIdentifier(
3303
function buildTemporaryPlace(builder: HIRBuilder, loc: SourceLocation): Place {
3304
const place: Place = {
3305
kind: "Identifier",
3304
- identifier: builder.makeTemporary(),
3306
+ identifier: builder.makeTemporary(loc),
3307
effect: Effect.Unknown,
3308
reactive: false,
3309
loc,
compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
+3
@@ -1144,6 +1144,7 @@ export type Identifier = {
1144
*/
1145
scope: ReactiveScope | null;
1146
type: Type;
1147
+ loc: SourceLocation;
1148
};
1149
1150
export type IdentifierName = ValidatedIdentifier | PromotedIdentifier;
@@ -1376,6 +1377,8 @@ export type ReactiveScope = {
1377
* no longer exist due to being pruned.
1378
*/
1379
merged: Set<ScopeId>;
1380
+
1381
+ loc: SourceLocation;
1382
};
1383
1384
export type ReactiveScopeDependencies = Set<ReactiveScopeDependency>;
compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts
+9
-2
@@ -21,6 +21,7 @@ import {
21
IdentifierId,
22
Instruction,
23
Place,
24
+ SourceLocation,
25
Terminal,
26
VariableBinding,
27
makeBlockId,
@@ -174,7 +175,7 @@ export default class HIRBuilder {
175
return handler ?? null;
176
}
177
177
- makeTemporary(): Identifier {
178
+ makeTemporary(loc: SourceLocation): Identifier {
179
const id = this.nextIdentifierId;
180
return {
181
id,
@@ -182,6 +183,7 @@ export default class HIRBuilder {
183
mutableRange: { start: makeInstructionId(0), end: makeInstructionId(0) },
184
scope: null,
185
type: makeType(),
186
+ loc,
187
};
188
}
189
@@ -320,6 +322,7 @@ export default class HIRBuilder {
322
},
323
scope: null,
324
type: makeType(),
325
+ loc: node.loc ?? GeneratedSource,
326
};
327
this.#bindings.set(name, { node, identifier });
328
return identifier;
@@ -877,7 +880,10 @@ export function removeUnnecessaryTryCatch(fn: HIR): void {
880
}
881
}
882
880
-export function createTemporaryPlace(env: Environment): Place {
883
+export function createTemporaryPlace(
884
+ env: Environment,
885
+ loc: SourceLocation
886
+): Place {
887
return {
888
kind: "Identifier",
889
identifier: {
@@ -886,6 +892,7 @@ export function createTemporaryPlace(env: Environment): Place {
892
name: null,
893
scope: null,
894
type: makeType(),
895
+ loc,
896
},
897
reactive: false,
898
effect: Effect.Unknown,
compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts
+2
-2
@@ -178,7 +178,7 @@ function makeManualMemoizationMarkers(
178
return [
179
{
180
id: makeInstructionId(0),
181
- lvalue: createTemporaryPlace(env),
181
+ lvalue: createTemporaryPlace(env, fnExpr.loc),
182
value: {
183
kind: "StartMemoize",
184
manualMemoId,
@@ -193,7 +193,7 @@ function makeManualMemoizationMarkers(
193
},
194
{
195
id: makeInstructionId(0),
196
- lvalue: createTemporaryPlace(env),
196
+ lvalue: createTemporaryPlace(env, fnExpr.loc),
197
value: {
198
kind: "FinishMemoize",
199
manualMemoId,
compiler/packages/babel-plugin-react-compiler/src/Inference/InlineImmediatelyInvokedFunctionExpressions.ts
+2
@@ -236,6 +236,7 @@ function rewriteBlock(
236
name: null,
237
scope: null,
238
type: makeType(),
239
+ loc: terminal.loc,
240
},
241
kind: "Identifier",
242
reactive: false,
@@ -277,6 +278,7 @@ function declareTemporary(
278
name: null,
279
scope: null,
280
type: makeType(),
281
+ loc: result.loc,
282
},
283
kind: "Identifier",
284
reactive: false,
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts
+6
@@ -597,6 +597,10 @@ function codegenReactiveScope(
597
cx.env.config.enableChangeDetectionForDebugging != null &&
598
changeExpressions.length > 0
599
) {
600
+ const loc =
601
+ typeof scope.loc === "symbol"
602
+ ? "unknown location"
603
+ : `(${scope.loc.start.line}:${scope.loc.end.line})`;
604
const detectionFunction =
605
cx.env.config.enableChangeDetectionForDebugging.importSpecifierName;
606
const cacheLoadOldValueStatements: Array<t.Statement> = [];
@@ -626,6 +630,7 @@ function codegenReactiveScope(
630
t.stringLiteral(name.name),
631
t.stringLiteral(cx.fnName),
632
t.stringLiteral("cached"),
633
+ t.stringLiteral(loc),
634
])
635
)
636
);
@@ -637,6 +642,7 @@ function codegenReactiveScope(
642
t.stringLiteral(name.name),
643
t.stringLiteral(cx.fnName),
644
t.stringLiteral("recomputed"),
645
+ t.stringLiteral(loc),
646
])
647
)
648
);
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts
+22
-1
@@ -5,7 +5,7 @@
5
* LICENSE file in the root directory of this source tree.
6
*/
7
8
-import { CompilerError } from "..";
8
+import { CompilerError, SourceLocation } from "..";
9
import { Environment } from "../HIR";
10
import {
11
GeneratedSource,
@@ -110,6 +110,7 @@ export function inferReactiveScopeVariables(fn: HIRFunction): void {
110
reassignments: new Set(),
111
earlyReturnValue: null,
112
merged: new Set(),
113
+ loc: identifier.loc,
114
};
115
scopes.set(groupIdentifier, scope);
116
} else {
@@ -119,6 +120,7 @@ export function inferReactiveScopeVariables(fn: HIRFunction): void {
120
scope.range.end = makeInstructionId(
121
Math.max(scope.range.end, identifier.mutableRange.end)
122
);
123
+ scope.loc = mergeLocation(scope.loc, identifier.loc);
124
}
125
identifier.scope = scope;
126
identifier.mutableRange = scope.range;
@@ -159,6 +161,25 @@ export function inferReactiveScopeVariables(fn: HIRFunction): void {
161
}
162
}
163
164
+function mergeLocation(l: SourceLocation, r: SourceLocation): SourceLocation {
165
+ if (l === GeneratedSource) {
166
+ return r;
167
+ } else if (r === GeneratedSource) {
168
+ return l;
169
+ } else {
170
+ return {
171
+ start: {
172
+ line: Math.min(l.start.line, r.start.line),
173
+ column: Math.min(l.start.column, r.start.column),
174
+ },
175
+ end: {
176
+ line: Math.max(l.end.line, r.end.line),
177
+ column: Math.max(l.end.column, r.end.column),
178
+ },
179
+ };
180
+ }
181
+}
182
+
183
// Is the operand mutable at this given instruction
184
export function isMutable({ id }: Instruction, place: Place): boolean {
185
const range = place.identifier.mutableRange;
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateEarlyReturns.ts
+5
-5
@@ -153,10 +153,10 @@ class Transform extends ReactiveFunctionTransform<State> {
153
154
const instructions = scopeBlock.instructions;
155
const loc = earlyReturnValue.loc;
156
- const sentinelTemp = createTemporaryPlace(this.env);
157
- const symbolTemp = createTemporaryPlace(this.env);
158
- const forTemp = createTemporaryPlace(this.env);
159
- const argTemp = createTemporaryPlace(this.env);
156
+ const sentinelTemp = createTemporaryPlace(this.env, loc);
157
+ const symbolTemp = createTemporaryPlace(this.env, loc);
158
+ const forTemp = createTemporaryPlace(this.env, loc);
159
+ const argTemp = createTemporaryPlace(this.env, loc);
160
scopeBlock.instructions = [
161
{
162
kind: "instruction",
@@ -274,7 +274,7 @@ class Transform extends ReactiveFunctionTransform<State> {
274
if (state.earlyReturnValue !== null) {
275
earlyReturnValue = state.earlyReturnValue;
276
} else {
277
- const identifier = createTemporaryPlace(this.env).identifier;
277
+ const identifier = createTemporaryPlace(this.env, loc).identifier;
278
promoteTemporary(identifier);
279
earlyReturnValue = {
280
label: this.env.nextBlockId,
compiler/packages/babel-plugin-react-compiler/src/SSA/EnterSSA.ts
+1
@@ -86,6 +86,7 @@ class SSABuilder {
86
},
87
scope: null, // reset along w the mutable range
88
type: makeType(),
89
+ loc: oldId.loc,
90
};
91
}
92
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/change-detect-reassign.expect.md
+2
-2
@@ -29,14 +29,14 @@ function Component(props) {
29
let condition = $[0] !== props.value;
30
if (!condition) {
31
let old$x = $[1];
32
- $structuralCheck(old$x, x, "x", "Component", "cached");
32
+ $structuralCheck(old$x, x, "x", "Component", "cached", "(3:6)");
33
}
34
$[0] = props.value;
35
$[1] = x;
36
if (condition) {
37
x = [];
38
x.push(props.value);
39
- $structuralCheck($[1], x, "x", "Component", "recomputed");
39
+ $structuralCheck($[1], x, "x", "Component", "recomputed", "(3:6)");
40
x = $[1];
41
}
42
}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-and-other-hook-unpruned-dependency.expect.md
+4
-4
@@ -46,13 +46,13 @@ function Component(props) {
46
let condition = $[0] !== props.x;
47
if (!condition) {
48
let old$t0 = $[1];
49
- $structuralCheck(old$t0, t0, "t0", "Component", "cached");
49
+ $structuralCheck(old$t0, t0, "t0", "Component", "cached", "(8:8)");
50
}
51
$[0] = props.x;
52
$[1] = t0;
53
if (condition) {
54
t0 = f(props.x);
55
- $structuralCheck($[1], t0, "t0", "Component", "recomputed");
55
+ $structuralCheck($[1], t0, "t0", "Component", "recomputed", "(8:8)");
56
t0 = $[1];
57
}
58
}
@@ -65,13 +65,13 @@ function Component(props) {
65
let condition = $[2] !== x;
66
if (!condition) {
67
let old$t1 = $[3];
68
- $structuralCheck(old$t1, t1, "t1", "Component", "cached");
68
+ $structuralCheck(old$t1, t1, "t1", "Component", "cached", "(11:11)");
69
}
70
$[2] = x;
71
$[3] = t1;
72
if (condition) {
73
t1 = <div>{x}</div>;
74
- $structuralCheck($[3], t1, "t1", "Component", "recomputed");
74
+ $structuralCheck($[3], t1, "t1", "Component", "recomputed", "(11:11)");
75
t1 = $[3];
76
}
77
}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-pruned-dependency-change-detect.expect.md
+2
-2
@@ -35,13 +35,13 @@ function Component(props) {
35
let condition = $[1] !== x;
36
if (!condition) {
37
let old$t1 = $[2];
38
- $structuralCheck(old$t1, t1, "t1", "Component", "cached");
38
+ $structuralCheck(old$t1, t1, "t1", "Component", "cached", "(6:6)");
39
}
40
$[1] = x;
41
$[2] = t1;
42
if (condition) {
43
t1 = <div>{x}</div>;
44
- $structuralCheck($[2], t1, "t1", "Component", "recomputed");
44
+ $structuralCheck($[2], t1, "t1", "Component", "recomputed", "(6:6)");
45
t1 = $[2];
46
}
47
}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-unpruned-dependency.expect.md
+4
-4
@@ -42,13 +42,13 @@ function Component(props) {
42
let condition = $[0] !== props.x;
43
if (!condition) {
44
let old$t0 = $[1];
45
- $structuralCheck(old$t0, t0, "t0", "Component", "cached");
45
+ $structuralCheck(old$t0, t0, "t0", "Component", "cached", "(4:4)");
46
}
47
$[0] = props.x;
48
$[1] = t0;
49
if (condition) {
50
t0 = f(props.x);
51
- $structuralCheck($[1], t0, "t0", "Component", "recomputed");
51
+ $structuralCheck($[1], t0, "t0", "Component", "recomputed", "(4:4)");
52
t0 = $[1];
53
}
54
}
@@ -65,7 +65,7 @@ function Component(props) {
65
let condition = $[2] !== x || $[3] !== w;
66
if (!condition) {
67
let old$t1 = $[4];
68
- $structuralCheck(old$t1, t1, "t1", "Component", "cached");
68
+ $structuralCheck(old$t1, t1, "t1", "Component", "cached", "(7:10)");
69
}
70
$[2] = x;
71
$[3] = w;
@@ -77,7 +77,7 @@ function Component(props) {
77
{w}
78
</div>
79
);
80
- $structuralCheck($[4], t1, "t1", "Component", "recomputed");
80
+ $structuralCheck($[4], t1, "t1", "Component", "recomputed", "(7:10)");
81
t1 = $[4];
82
}
83
}
compiler/packages/react-compiler-runtime/src/index.ts
+4
-3
@@ -259,10 +259,11 @@ export function $structuralCheck(
259
newValue: any,
260
variableName: string,
261
fnName: string,
262
- kind: string
262
+ kind: string,
263
+ loc: string
264
): void {
265
function error(l: string, r: string, path: string, depth: number) {
265
- const str = `${fnName}: [${kind}] ${variableName}${path} changed from ${l} to ${r} at depth ${depth}`;
266
+ const str = `${fnName}:${loc} [${kind}] ${variableName}${path} changed from ${l} to ${r} at depth ${depth}`;
267
if (seenErrors.has(str)) {
268
return;
269
}
@@ -283,7 +284,7 @@ export function $structuralCheck(
284
if (oldValue === null && newValue !== null) {
285
error("null", `type ${typeof newValue}`, path, depth);
286
} else if (newValue === null) {
286
- error(`type ${typeof oldValue}`, null, path, depth);
287
+ error(`type ${typeof oldValue}`, "null", path, depth);
288
} else if (oldValue instanceof Map) {
289
if (!(newValue instanceof Map)) {
290
error(`Map instance`, `other value`, path, depth);