Fix mutable range validation in PrintHIR
Fixes the mutable range validation in PrintHIR: when we checked the identifier scope's range we failed to allow end=start=0 cases which are also valid. While I was here, I created a separate assertion pass to check all ranges, that way we aren't relying on the printer to validate them. The pass is on for playground and tests, but disabled by default so it can't break internal apps.
Joe Savona committed
Jun 21, 2023 at 16:08 UTC
ba75c96077494248e8c877cc3c403372d866b3e9
9 files changed
+125
-14
compiler/forget/apps/playground/components/Editor/index.tsx
+1
@@ -127,6 +127,7 @@ function compile(source: string): CompilerOutput {
127
inlineUseMemo: true,
128
memoizeJsxElements: true,
129
validateHooksUsage: true,
130
+ assertValidMutableRanges: true,
131
})) {
132
const fnName = fn.node.id?.name ?? null;
133
switch (result.kind) {
compiler/forget/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts
+5
@@ -12,6 +12,7 @@ import {
12
ReactiveFunction,
13
assertConsistentIdentifiers,
14
assertTerminalSuccessorsExist,
15
+ assertValidMutableRanges,
16
lower,
17
mergeConsecutiveBlocks,
18
} from "../HIR";
@@ -127,6 +128,10 @@ export function* run(
128
inferMutableRanges(hir);
129
yield log({ kind: "hir", name: "InferMutableRanges", value: hir });
130
131
+ if (env.assertValidMutableRanges) {
132
+ assertValidMutableRanges(hir);
133
+ }
134
+
135
if (env.validateRefAccessDuringRender) {
136
validateNoRefAccessInRender(hir);
137
}
compiler/forget/packages/babel-plugin-react-forget/src/HIR/AssertValidMutableRanges.ts
new
+56
@@ -0,0 +1,56 @@
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 invariant from "invariant";
9
+import { HIRFunction, Identifier, MutableRange } from "./HIR";
10
+import {
11
+ eachInstructionLValue,
12
+ eachInstructionOperand,
13
+ eachTerminalOperand,
14
+} from "./visitors";
15
+
16
+/**
17
+ * Checks that all mutable ranges in the function are well-formed, with
18
+ * start === end === 0 OR end > start.
19
+ */
20
+export function assertValidMutableRanges(fn: HIRFunction): void {
21
+ for (const [, block] of fn.body.blocks) {
22
+ for (const phi of block.phis) {
23
+ for (const [, operand] of phi.operands) {
24
+ visitIdentifier(operand);
25
+ }
26
+ }
27
+ for (const instr of block.instructions) {
28
+ for (const operand of eachInstructionLValue(instr)) {
29
+ visitIdentifier(operand.identifier);
30
+ }
31
+ for (const operand of eachInstructionOperand(instr)) {
32
+ visitIdentifier(operand.identifier);
33
+ }
34
+ }
35
+ for (const operand of eachTerminalOperand(block.terminal)) {
36
+ visitIdentifier(operand.identifier);
37
+ }
38
+ }
39
+}
40
+
41
+function visitIdentifier(identifier: Identifier): void {
42
+ validateMutableRange(identifier.mutableRange);
43
+ if (identifier.scope !== null) {
44
+ validateMutableRange(identifier.scope.range);
45
+ }
46
+}
47
+
48
+function validateMutableRange(mutableRange: MutableRange): void {
49
+ invariant(
50
+ (mutableRange.start === 0 && mutableRange.end === 0) ||
51
+ mutableRange.end > mutableRange.start,
52
+ "Identifier scope mutableRange was invalid: [%s:%s]",
53
+ mutableRange.start,
54
+ mutableRange.end
55
+ );
56
+}
compiler/forget/packages/babel-plugin-react-forget/src/HIR/Environment.ts
+9
@@ -159,6 +159,13 @@ export type EnvironmentConfig = Partial<{
159
* Defaults to false (use the un-transformed function body).
160
*/
161
enableOptimizeFunctionExpressions: boolean;
162
+
163
+ /**
164
+ * Enable validation of mutable ranges
165
+ *
166
+ * Defaults to false
167
+ */
168
+ assertValidMutableRanges: boolean;
169
}>;
170
171
export class Environment {
@@ -175,6 +182,7 @@ export class Environment {
182
disableAllMemoization: boolean;
183
enableEmitFreeze: ExternalFunction | null;
184
enableOptimizeFunctionExpressions: boolean;
185
+ assertValidMutableRanges: boolean;
186
187
#contextIdentifiers: Set<t.Identifier>;
188
@@ -220,6 +228,7 @@ export class Environment {
228
this.enableEmitFreeze = config?.enableEmitFreeze ?? null;
229
this.enableOptimizeFunctionExpressions =
230
config?.enableOptimizeFunctionExpressions ?? false;
231
+ this.assertValidMutableRanges = config?.assertValidMutableRanges ?? false;
232
233
this.#contextIdentifiers = contextIdentifiers;
234
}
compiler/forget/packages/babel-plugin-react-forget/src/HIR/PrintHIR.ts
-14
@@ -523,20 +523,6 @@ function isMutable(range: MutableRange): boolean {
523
}
524
525
function printMutableRange(identifier: Identifier): string {
526
- invariant(
527
- (identifier.mutableRange.start === 0 &&
528
- identifier.mutableRange.end === 0) ||
529
- identifier.mutableRange.end > identifier.mutableRange.start,
530
- "Identifier mutableRange was invalid: [%s:%s]",
531
- identifier.mutableRange.start,
532
- identifier.mutableRange.end
533
- );
534
- if (identifier.scope !== null) {
535
- invariant(
536
- identifier.scope.range.end > identifier.scope.range.start,
537
- "Identifier scope mutableRange was invalid"
538
- );
539
- }
526
const range =
527
identifier.scope !== null
528
? identifier.scope.range
compiler/forget/packages/babel-plugin-react-forget/src/HIR/index.ts
+1
@@ -7,6 +7,7 @@
7
8
export { assertConsistentIdentifiers } from "./AssertConsistentIdentifiers";
9
export { assertTerminalSuccessorsExist } from "./AssertTerminalSuccessorsExist";
10
+export { assertValidMutableRanges } from "./AssertValidMutableRanges";
11
export { lower } from "./BuildHIR";
12
export { computeDominatorTree, computePostDominatorTree } from "./Dominator";
13
export { Environment, Hook } from "./Environment";
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md
new
+42
@@ -0,0 +1,42 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function HomeDiscoStoreItemTileRating(props) {
6
+ const item = useFragment();
7
+ let count = 0;
8
+ const aggregates = item?.aggregates || [];
9
+ aggregates.forEach((aggregate) => {
10
+ count += aggregate.count || 0;
11
+ });
12
+
13
+ return <Text>{count}</Text>;
14
+}
15
+
16
+```
17
+
18
+## Code
19
+
20
+```javascript
21
+import { unstable_useMemoCache as useMemoCache } from "react";
22
+function HomeDiscoStoreItemTileRating(props) {
23
+ const $ = useMemoCache(1);
24
+ const item = useFragment();
25
+ let count;
26
+ count = 0;
27
+ const aggregates = item?.aggregates || [];
28
+ aggregates.forEach((aggregate) => {
29
+ count += aggregate.count || 0;
30
+ });
31
+ let t0;
32
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
33
+ t0 = <Text>{count}</Text>;
34
+ $[0] = t0;
35
+ } else {
36
+ t0 = $[0];
37
+ }
38
+ return t0;
39
+}
40
+
41
+```
42
+
\ No newline at end of file
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.js
new
+10
@@ -0,0 +1,10 @@
1
+function HomeDiscoStoreItemTileRating(props) {
2
+ const item = useFragment();
3
+ let count = 0;
4
+ const aggregates = item?.aggregates || [];
5
+ aggregates.forEach((aggregate) => {
6
+ count += aggregate.count || 0;
7
+ });
8
+
9
+ return <Text>{count}</Text>;
10
+}
compiler/forget/packages/snap/src/compiler-worker.ts
+1
@@ -167,6 +167,7 @@ export async function compile(
167
validateFrozenLambdas: true,
168
enableEmitFreeze,
169
enableOptimizeFunctionExpressions,
170
+ assertValidMutableRanges: true,
171
},
172
logger: null,
173
gating,