[DeriveMinimalDeps] Account for conditional / poisoned accesses within conditionals
[DeriveMinimalDeps] Account for conditional / poisoned accesses within conditionals This change is needed for #2752. To minimize renaming `error.fixture` -> `fixture` files, I'm reordering this PR to earlier in the stack. Prior to the fix in #2752, we only expected unconditional accesses within `depsInCurrentConditional`, which records instructions directly within a conditional block (not including nested conditional blocks).
Mofei Zhang committed
Mar 27, 2024 at 20:26 UTC
b98b569017c1fd896159ba2430faadcf03d7bf70
1 file changed
+27
-30
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/DeriveMinimalDependencies.ts
+27
-30
@@ -6,7 +6,7 @@
6
*/
7
8
import { CompilerError } from "../CompilerError";
9
-import { GeneratedSource, Identifier, ReactiveScopeDependency } from "../HIR";
9
+import { Identifier, ReactiveScopeDependency } from "../HIR";
10
import { printIdentifier } from "../HIR/PrintHIR";
11
import { assertExhaustive } from "../Utils/utils";
12
@@ -182,11 +182,18 @@ export class ReactiveScopeDependencyTree {
182
});
183
184
for (const [id, root] of this.#roots) {
185
- const nodesForRootId = mapNonNull(trees, (tree) => tree.#roots.get(id));
185
+ const nodesForRootId = mapNonNull(trees, (tree) => {
186
+ const node = tree.#roots.get(id);
187
+ if (node != null && isUnconditional(node.accessType)) {
188
+ return node;
189
+ } else {
190
+ return null;
191
+ }
192
+ });
193
if (nodesForRootId) {
194
addSubtreeIntersection(
188
- nodesForRootId.map((root) => root.properties),
189
- root.properties
195
+ root.properties,
196
+ nodesForRootId.map((root) => root.properties)
197
);
198
}
199
}
@@ -470,17 +477,18 @@ function addSubtree(
477
* dependency in at least one branch (otherwise `UnconditionalAccess`)
478
*
479
* @param otherProperties (read-only) an array of node properties containing
473
- * only unconditionally accessed nodes. Each element represents a
474
- * subtree of reactive dependencies from a single CFG branch.
475
- * otherProperties must represent all reachable branches.
480
+ * conditionally and unconditionally accessed nodes. Each element
481
+ * represents asubtree of reactive dependencies from a single CFG
482
+ * branch.
483
+ * otherProperties must represent all reachable branches.
484
* @param currProperties (mutable) return by argument properties of a node
485
*
486
* otherProperties and currProperties must be properties of disjoint nodes
487
* that represent the same dependency (identifier + path).
488
*/
489
function addSubtreeIntersection(
482
- otherProperties: Array<Map<string, DependencyNode>>,
483
- currProperties: Map<string, DependencyNode>
490
+ currProperties: Map<string, DependencyNode>,
491
+ otherProperties: Array<Map<string, DependencyNode>>
492
): void {
493
CompilerError.invariant(otherProperties.length > 1, {
494
reason:
@@ -490,22 +498,6 @@ function addSubtreeIntersection(
498
suggestions: null,
499
});
500
493
- CompilerError.invariant(
494
- otherProperties.every((otherNode) => {
495
- for (const [_, node] of otherNode) {
496
- if (!isUnconditional(node.accessType)) {
497
- return false;
498
- }
499
- }
500
- return true;
501
- }),
502
- {
503
- reason:
504
- "[DeriveMinimalDependencies] Expected otherProperties to only hold unconditional nodes",
505
- loc: GeneratedSource,
506
- }
507
- );
508
-
501
/*
502
* otherProperties here may contain unconditional nodes as the result of
503
* recursively merging exhaustively conditional children with unconditionally
@@ -514,9 +506,14 @@ function addSubtreeIntersection(
506
*/
507
508
for (const [propertyName, currNode] of currProperties) {
517
- const otherNodes = mapNonNull(otherProperties, (properties) =>
518
- properties.get(propertyName)
519
- );
509
+ const otherNodes = mapNonNull(otherProperties, (properties) => {
510
+ const node = properties.get(propertyName);
511
+ if (node != null && isUnconditional(node.accessType)) {
512
+ return node;
513
+ } else {
514
+ return null;
515
+ }
516
+ });
517
518
/*
519
* intersection(otherNodes[propertyName]) only exists if each element in
@@ -524,8 +521,8 @@ function addSubtreeIntersection(
521
*/
522
if (otherNodes) {
523
addSubtreeIntersection(
527
- otherNodes.map((node) => node.properties),
528
- currNode.properties
524
+ currNode.properties,
525
+ otherNodes.map((node) => node.properties)
526
);
527
528
const isDep = otherNodes.some((tree) => isDependency(tree.accessType));