ValidateUnconditionalHooks pass using dominators
See the code comments for more, but the basic idea here is that we use the post dominator tree to find the set of basic blocks which are guaranteed reachable in each function. Those are the only blocks where it is safe to call hooks, and we error for hook calls in any other blocks.
Joe Savona committed
May 8, 2023 at 08:55 UTC
6bbe3111c08c5c00a37f12aa6c1dc1f086a8b938
14 files changed
+279
-37
compiler/forget/src/CompilerPipeline.ts
+2
@@ -15,6 +15,7 @@ import {
15
validateConsistentIdentifiers,
16
validateHooksUsage,
17
validateTerminalSuccessors,
18
+ validateUnconditionalHooks,
19
} from "./HIR";
20
import { Environment, EnvironmentConfig } from "./HIR/Environment";
21
import {
@@ -87,6 +88,7 @@ export function* run(
88
89
if (env.validateHooksUsage) {
90
validateHooksUsage(hir);
91
+ validateUnconditionalHooks(hir);
92
}
93
94
dropMemoCalls(hir);
compiler/forget/src/HIR/ValidateUnconditionalHooks.ts
new
+101
@@ -0,0 +1,101 @@
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 {
9
+ CompilerError,
10
+ CompilerErrorDetail,
11
+ ErrorSeverity,
12
+} from "../CompilerError";
13
+import { findBlocksWithBackEdges } from "../Optimization/DeadCodeElimination";
14
+import { computeDominators } from "./Dominator";
15
+import { BlockId, HIRFunction, isHookType } from "./HIR";
16
+
17
+/**
18
+ * Validates that the function honors the [Rules of Hooks](https://react.dev/warnings/invalid-hook-call-warning)
19
+ * rule that hooks may not be called conditionally. More precisely, a component or hook must always call the
20
+ * same set of hooks in the same order.
21
+ *
22
+ * The algorithm is based on [Dominators](https://en.wikipedia.org/wiki/Dominator_(graph_theory)). Hooks may
23
+ * only be called in basic blocks that are unconditionally reachable from the entry node. In graph theory,
24
+ * this corresponds to basic blocks which post dominate the entry block — that are on every path from the
25
+ * entry block to the exit:
26
+ *
27
+ * ```
28
+ * bb0 (entry)
29
+ * / \
30
+ * bb1 bb2
31
+ * \ /
32
+ * bb3
33
+ * |
34
+ * (exit)
35
+ * ```
36
+ *
37
+ * Here, neither bb1 or bb2 post dominate the entry, which corresponds to the fact that control can
38
+ * flow from the entry node to either of these nodes. However, bb3 does post dominate the entry node:
39
+ * control flow will _always_ reach bb3 from the entry node. In this graph is is therefore safe to call
40
+ * hooks only in bb0 and bb3, the post dominators of bb0.
41
+ *
42
+ * However if for example bb2 were to early return:
43
+ *
44
+ * ```
45
+ * bb0 (entry)
46
+ * / \
47
+ * bb1 bb2
48
+ * \ |
49
+ * bb3 /
50
+ * | /
51
+ * (exit)
52
+ * ```
53
+ *
54
+ * Now only the exit node would post dominate the entry node: there is no other node which is
55
+ * guaranteed to be reachable. In this graph is is only safe to call hooks in bb0.
56
+ */
57
+export function validateUnconditionalHooks(fn: HIRFunction): void {
58
+ // Construct the set of blocks that is always reachable from the entry block.
59
+ const unconditionalBlocks = new Set<BlockId>();
60
+ const blocksWithBackEdges = findBlocksWithBackEdges(fn);
61
+ const dominators = computeDominators(fn, { reverse: true });
62
+ // Post dominator graph so .entry is the "exit" node
63
+ const exit = dominators.entry;
64
+ let current: BlockId | null = fn.body.entry;
65
+ while (
66
+ current !== null &&
67
+ current !== exit &&
68
+ !blocksWithBackEdges.has(current)
69
+ ) {
70
+ unconditionalBlocks.add(current);
71
+ current = dominators.get(current);
72
+ }
73
+
74
+ const errors = new CompilerError();
75
+ for (const [, block] of fn.body.blocks) {
76
+ if (unconditionalBlocks.has(block.id)) {
77
+ continue;
78
+ }
79
+ for (const instr of block.instructions) {
80
+ if (
81
+ instr.value.kind === "CallExpression" &&
82
+ isHookType(instr.value.callee.identifier)
83
+ ) {
84
+ const loc = instr.loc;
85
+ errors.pushErrorDetail(
86
+ new CompilerErrorDetail({
87
+ codeframe: null,
88
+ description: null,
89
+ reason:
90
+ "Hooks must always be called in a consistent order, and may not be called conditionally. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning)",
91
+ loc: typeof loc !== "symbol" ? loc : null,
92
+ severity: ErrorSeverity.InvalidInput,
93
+ })
94
+ );
95
+ }
96
+ }
97
+ }
98
+ if (errors.hasErrors()) {
99
+ throw errors;
100
+ }
101
+}
compiler/forget/src/HIR/index.ts
+1
@@ -21,3 +21,4 @@ export { printFunction, printHIR } from "./PrintHIR";
21
export { validateConsistentIdentifiers } from "./ValidateConsistentIdentifiers";
22
export { validateHooksUsage } from "./ValidateHooksUsage";
23
export { validateTerminalSuccessors } from "./ValidateTerminalSuccessors";
24
+export { validateUnconditionalHooks } from "./ValidateUnconditionalHooks";
compiler/forget/src/Optimization/DeadCodeElimination.ts
+7
-2
@@ -248,14 +248,19 @@ function pruneableValue(value: InstructionValue, state: State): boolean {
248
}
249
250
export function hasBackEdge(fn: HIRFunction): boolean {
251
+ return findBlocksWithBackEdges(fn).size > 0;
252
+}
253
+
254
+export function findBlocksWithBackEdges(fn: HIRFunction): Set<BlockId> {
255
const visited = new Set<BlockId>();
256
+ const blocks = new Set<BlockId>();
257
for (const [blockId, block] of fn.body.blocks) {
258
for (const predId of block.preds) {
259
if (!visited.has(predId)) {
255
- return true;
260
+ blocks.add(blockId);
261
}
262
}
263
visited.add(blockId);
264
}
260
- return false;
265
+ return blocks;
266
}
compiler/forget/src/__tests__/fixtures/compiler/dominator.expect.md
+32
-18
@@ -2,7 +2,6 @@
2
## Input
3
4
```javascript
5
-// @only @debug
5
function Component(props) {
6
let x = 0;
7
label: if (props.a) {
@@ -15,22 +14,22 @@ function Component(props) {
14
}
15
x = 3;
16
}
18
- // label2: switch (props.c) {
19
- // case "a": {
20
- // x = 4;
21
- // break;
22
- // }
23
- // case "b": {
24
- // break label2;
25
- // }
26
- // case "c": {
27
- // x = 5;
28
- // // intentional fallthrough
29
- // }
30
- // default: {
31
- // x = 6;
32
- // }
33
- // }
17
+ label2: switch (props.c) {
18
+ case "a": {
19
+ x = 4;
20
+ break;
21
+ }
22
+ case "b": {
23
+ break label2;
24
+ }
25
+ case "c": {
26
+ x = 5;
27
+ // intentional fallthrough
28
+ }
29
+ default: {
30
+ x = 6;
31
+ }
32
+ }
33
if (props.d) {
34
return null;
35
}
@@ -42,7 +41,6 @@ function Component(props) {
41
## Code
42
43
```javascript
45
-// @only @debug
44
function Component(props) {
45
let x = 0;
46
if (props.a) {
@@ -53,6 +51,22 @@ function Component(props) {
51
} else {
52
}
53
}
54
+ bb10: {
55
+ switch (props.c) {
56
+ case "a": {
57
+ x = 4;
58
+ break bb10;
59
+ }
60
+ case "b": {
61
+ break bb10;
62
+ }
63
+ case "c": {
64
+ }
65
+ default: {
66
+ x = 6;
67
+ }
68
+ }
69
+ }
70
if (props.d) {
71
return null;
72
}
compiler/forget/src/__tests__/fixtures/compiler/dominator.js
+16
-17
@@ -1,4 +1,3 @@
1
-// @only @debug
1
function Component(props) {
2
let x = 0;
3
label: if (props.a) {
@@ -11,22 +10,22 @@ function Component(props) {
10
}
11
x = 3;
12
}
14
- // label2: switch (props.c) {
15
- // case "a": {
16
- // x = 4;
17
- // break;
18
- // }
19
- // case "b": {
20
- // break label2;
21
- // }
22
- // case "c": {
23
- // x = 5;
24
- // // intentional fallthrough
25
- // }
26
- // default: {
27
- // x = 6;
28
- // }
29
- // }
13
+ label2: switch (props.c) {
14
+ case "a": {
15
+ x = 4;
16
+ break;
17
+ }
18
+ case "b": {
19
+ break label2;
20
+ }
21
+ case "c": {
22
+ x = 5;
23
+ // intentional fallthrough
24
+ }
25
+ default: {
26
+ x = 6;
27
+ }
28
+ }
29
if (props.d) {
30
return null;
31
}
compiler/forget/src/__tests__/fixtures/compiler/error.invalid-hook-after-early-return.expect.md
new
+21
@@ -0,0 +1,21 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ if (props.cond) {
7
+ return null;
8
+ }
9
+ return useHook();
10
+}
11
+
12
+```
13
+
14
+
15
+## Error
16
+
17
+```
18
+[ReactForget] InvalidInput: Hooks must always be called in a consistent order, and may not be called conditionally. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (5:5)
19
+```
20
+
21
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/error.invalid-hook-after-early-return.js
new
+6
@@ -0,0 +1,6 @@
1
+function Component(props) {
2
+ if (props.cond) {
3
+ return null;
4
+ }
5
+ return useHook();
6
+}
compiler/forget/src/__tests__/fixtures/compiler/error.invalid-hook-for.expect.md
new
+26
@@ -0,0 +1,26 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ let i = 0;
7
+ for (let x = 0; useHook(x) < 10; useHook(i), x++) {
8
+ i += useHook(x);
9
+ }
10
+ return i;
11
+}
12
+
13
+```
14
+
15
+
16
+## Error
17
+
18
+```
19
+[ReactForget] InvalidInput: Hooks must always be called in a consistent order, and may not be called conditionally. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (3:3)
20
+
21
+[ReactForget] InvalidInput: Hooks must always be called in a consistent order, and may not be called conditionally. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (4:4)
22
+
23
+[ReactForget] InvalidInput: Hooks must always be called in a consistent order, and may not be called conditionally. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (3:3)
24
+```
25
+
26
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/error.invalid-hook-for.js
new
+7
@@ -0,0 +1,7 @@
1
+function Component(props) {
2
+ let i = 0;
3
+ for (let x = 0; useHook(x) < 10; useHook(i), x++) {
4
+ i += useHook(x);
5
+ }
6
+ return i;
7
+}
compiler/forget/src/__tests__/fixtures/compiler/error.invalid-hook-if-alternate.expect.md
new
+23
@@ -0,0 +1,23 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ let x = null;
7
+ if (props.cond) {
8
+ } else {
9
+ x = useHook();
10
+ }
11
+ return x;
12
+}
13
+
14
+```
15
+
16
+
17
+## Error
18
+
19
+```
20
+[ReactForget] InvalidInput: Hooks must always be called in a consistent order, and may not be called conditionally. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (5:5)
21
+```
22
+
23
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/error.invalid-hook-if-alternate.js
new
+8
@@ -0,0 +1,8 @@
1
+function Component(props) {
2
+ let x = null;
3
+ if (props.cond) {
4
+ } else {
5
+ x = useHook();
6
+ }
7
+ return x;
8
+}
compiler/forget/src/__tests__/fixtures/compiler/error.invalid-hook-if-consequent.expect.md
new
+22
@@ -0,0 +1,22 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ let x = null;
7
+ if (props.cond) {
8
+ x = useHook();
9
+ }
10
+ return x;
11
+}
12
+
13
+```
14
+
15
+
16
+## Error
17
+
18
+```
19
+[ReactForget] InvalidInput: Hooks must always be called in a consistent order, and may not be called conditionally. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (4:4)
20
+```
21
+
22
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/error.invalid-hook-if-consequent.js
new
+7
@@ -0,0 +1,7 @@
1
+function Component(props) {
2
+ let x = null;
3
+ if (props.cond) {
4
+ x = useHook();
5
+ }
6
+ return x;
7
+}