From d909ae04349b5a781409929acd18447e80ada4e9 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 14 Dec 2022 08:42:38 -0800 Subject: [PATCH] Fix scope dependency collection ordering bug Dependency collection has to visit the instruction id first before evaluating the instruction, in order to completely any scopes that would end at that instruction. Note the removed dependencies that don't appear within the scopes. --- .../forget/src/HIR/InferReactiveScopeDependencies.ts | 3 +-- .../src/__tests__/fixtures/hir/hook-call.expect.md | 3 +-- .../hir/overlapping-scopes-shadowed.expect.md | 7 +++---- .../fixtures/hir/reactive-scope-grouping.expect.md | 9 +++------ .../fixtures/hir/reassignment-conditional.expect.md | 4 +--- .../__tests__/fixtures/hir/reassignment.expect.md | 7 +++---- .../fixtures/hir/ssa-property-alias-if.expect.md | 12 +++--------- .../ssa-property-alias-mutate-inside-if.expect.md | 7 ++----- .../fixtures/hir/switch-non-final-default.expect.md | 4 +--- .../src/__tests__/fixtures/hir/switch.expect.md | 4 +--- .../fixtures/hir/type-binary-operator.expect.md | 5 ++--- .../fixtures/hir/type-test-field-store.expect.md | 9 +++------ .../hir/type-test-return-type-inference.expect.md | 5 ++--- 13 files changed, 26 insertions(+), 53 deletions(-) diff --git a/compiler/forget/src/HIR/InferReactiveScopeDependencies.ts b/compiler/forget/src/HIR/InferReactiveScopeDependencies.ts index 05f7bd6b80..a5385a52e1 100644 --- a/compiler/forget/src/HIR/InferReactiveScopeDependencies.ts +++ b/compiler/forget/src/HIR/InferReactiveScopeDependencies.ts @@ -127,6 +127,7 @@ class ScopeDependenciesVisitor } visitInstruction(instr: Instruction, _value: InstructionValue): void { + this.#visitId(instr.id); const { lvalue, value } = instr; if (lvalue !== null && lvalue.place.memberPath === null) { if (!this.#identifiers.has(lvalue.place.identifier)) { @@ -173,8 +174,6 @@ class ScopeDependenciesVisitor } } } - - this.#visitId(instr.id); } enterBlock(): void {} diff --git a/compiler/forget/src/__tests__/fixtures/hir/hook-call.expect.md b/compiler/forget/src/__tests__/fixtures/hir/hook-call.expect.md index 365db2ff7d..5172e0398d 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/hook-call.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/hook-call.expect.md @@ -81,7 +81,6 @@ bb0: [8] Return read $16_@2 scope1 [2:3]: - dependency: freeze x$11_@0 - - dependency: read x$11_@0 scope2 [7:8]: - dependency: read Component$0 - dependency: read $13 @@ -100,7 +99,7 @@ function Component( scope @0 [1:2] deps=[] { [1] Const mutate x$11_@0 = Array [] } - scope @1 [2:3] deps=[freeze x$11_@0, read x$11_@0] { + scope @1 [2:3] deps=[freeze x$11_@0] { [2] Const mutate y$12_@1 = Call read useFreeze$4:TFunction(freeze x$11_@0) } [3] Call mutate foo$5:TFunction(read y$12_@1, read x$11_@0) diff --git a/compiler/forget/src/__tests__/fixtures/hir/overlapping-scopes-shadowed.expect.md b/compiler/forget/src/__tests__/fixtures/hir/overlapping-scopes-shadowed.expect.md index 81f93b377b..caa5606488 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/overlapping-scopes-shadowed.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/overlapping-scopes-shadowed.expect.md @@ -21,11 +21,10 @@ bb0: [4] Call mutate x$7_@0.push(read a$5) [5] Return scope0 [1:5]: - - dependency: read b$6 - dependency: read a$5 + - dependency: read b$6 scope1 [2:4]: - dependency: read b$6 - - dependency: read a$5 ``` ## Reactive Scopes @@ -35,9 +34,9 @@ function foo( a, b, ) { - scope @0 [1:5] deps=[read b$6, read a$5] { + scope @0 [1:5] deps=[read a$5, read b$6] { [1] Const mutate x$7_@0:TFunction[1:5] = Array [] - scope @1 [2:4] deps=[read b$6, read a$5] { + scope @1 [2:4] deps=[read b$6] { [2] Const mutate y$8_@1:TFunction[2:4] = Array [] [3] Call mutate y$8_@1.push(read b$6) } diff --git a/compiler/forget/src/__tests__/fixtures/hir/reactive-scope-grouping.expect.md b/compiler/forget/src/__tests__/fixtures/hir/reactive-scope-grouping.expect.md index ceffb04a01..946e0db868 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/reactive-scope-grouping.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/reactive-scope-grouping.expect.md @@ -24,10 +24,7 @@ bb0: [4] Call mutate y$5_@1.push(mutate z$6_@1:TObject) [5] Reassign mutate x$4_@0.y[1:6] = read y$5_@1:TFunction [6] Return freeze x$4_@0:TObject -scope0 [1:6]: - - dependency: mutate x$4_@0.y -scope1 [2:5]: - - dependency: mutate x$4_@0.y + ``` ## Reactive Scopes @@ -35,9 +32,9 @@ scope1 [2:5]: ``` function foo( ) { - scope @0 [1:6] deps=[mutate x$4_@0.y] { + scope @0 [1:6] deps=[] { [1] Const mutate x$4_@0:TObject[1:6] = Object { } - scope @1 [2:5] deps=[mutate x$4_@0.y] { + scope @1 [2:5] deps=[] { [2] Const mutate y$5_@1:TFunction[2:5] = Array [] [3] Const mutate z$6_@1:TObject[2:5] = Object { } [4] Call mutate y$5_@1.push(mutate z$6_@1:TObject) diff --git a/compiler/forget/src/__tests__/fixtures/hir/reassignment-conditional.expect.md b/compiler/forget/src/__tests__/fixtures/hir/reassignment-conditional.expect.md index 38bb1a92d2..dc16c3fa9f 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/reassignment-conditional.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/reassignment-conditional.expect.md @@ -43,8 +43,6 @@ scope0 [1:7]: scope1 [7:8]: - dependency: read Component$0 - dependency: freeze x$7_@0:TFunction - - dependency: read y$8.push - - dependency: read props$6.p2 scope2 [9:10]: - dependency: read Component$0 - dependency: read x$7_@0:TFunction @@ -65,7 +63,7 @@ function Component( [5] Reassign mutate x$7_@0:TFunction[1:7] = Array [] } } - scope @1 [7:8] deps=[read Component$0, freeze x$7_@0:TFunction, read y$8.push, read props$6.p2] { + scope @1 [7:8] deps=[read Component$0, freeze x$7_@0:TFunction] { [7] Const mutate _$12_@1 = JSX } [8] Call read y$8.push(read props$6.p2) diff --git a/compiler/forget/src/__tests__/fixtures/hir/reassignment.expect.md b/compiler/forget/src/__tests__/fixtures/hir/reassignment.expect.md index 44c6af6f4c..320c519184 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/reassignment.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/reassignment.expect.md @@ -31,13 +31,12 @@ bb0: [8] Return read $11_@3 scope0 [1:7]: - dependency: read props$6.p0 + - dependency: read props$6.p1 - dependency: read Component$0 - dependency: freeze x$9_@1 - - dependency: read props$6.p1 scope2 [5:6]: - dependency: read Component$0 - dependency: freeze x$9_@1 - - dependency: read props$6.p1 scope3 [7:8]: - dependency: read Component$0 - dependency: read x$9_@1 @@ -50,14 +49,14 @@ scope3 [7:8]: function Component( props, ) { - scope @0 [1:7] deps=[read props$6.p0, read Component$0, freeze x$9_@1, read props$6.p1] { + scope @0 [1:7] deps=[read props$6.p0, read props$6.p1, read Component$0, freeze x$9_@1] { [1] Const mutate x$7_@0:TFunction[1:7] = Array [] [2] Call mutate x$7_@0.push(read props$6.p0) [3] Const mutate y$8_@0:TFunction[1:7] = read x$7_@0:TFunction scope @1 [4:5] deps=[] { [4] Const mutate x$9_@1 = Array [] } - scope @2 [5:6] deps=[read Component$0, freeze x$9_@1, read props$6.p1] { + scope @2 [5:6] deps=[read Component$0, freeze x$9_@1] { [5] Const mutate _$10_@2 = JSX } [6] Call mutate y$8_@0.push(read props$6.p1) diff --git a/compiler/forget/src/__tests__/fixtures/hir/ssa-property-alias-if.expect.md b/compiler/forget/src/__tests__/fixtures/hir/ssa-property-alias-if.expect.md index 0cfd14f33c..69d9eff800 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/ssa-property-alias-if.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/ssa-property-alias-if.expect.md @@ -37,12 +37,6 @@ bb1: [9] Return freeze x$6_@0:TObject scope0 [1:9]: - dependency: read a$5 - - dependency: mutate x$6_@0.y - - dependency: mutate x$6_@0.z -scope1 [3:4]: - - dependency: mutate x$6_@0.y -scope2 [6:7]: - - dependency: mutate x$6_@0.z ``` ## Reactive Scopes @@ -51,15 +45,15 @@ scope2 [6:7]: function foo( a, ) { - scope @0 [1:9] deps=[read a$5, mutate x$6_@0.y, mutate x$6_@0.z] { + scope @0 [1:9] deps=[read a$5] { [1] Const mutate x$6_@0:TObject[1:9] = Object { } if (read a$5) { - scope @1 [3:4] deps=[mutate x$6_@0.y] { + scope @1 [3:4] deps=[] { [3] Const mutate y$7_@1:TObject = Object { } } [4] Reassign mutate x$6_@0.y[1:9] = read y$7_@1:TObject } else { - scope @2 [6:7] deps=[mutate x$6_@0.z] { + scope @2 [6:7] deps=[] { [6] Const mutate z$8_@2:TObject = Object { } } [7] Reassign mutate x$6_@0.z[1:9] = read z$8_@2:TObject diff --git a/compiler/forget/src/__tests__/fixtures/hir/ssa-property-alias-mutate-inside-if.expect.md b/compiler/forget/src/__tests__/fixtures/hir/ssa-property-alias-mutate-inside-if.expect.md index 29ecf9a2a5..175da4727e 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/ssa-property-alias-mutate-inside-if.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/ssa-property-alias-mutate-inside-if.expect.md @@ -39,9 +39,6 @@ bb1: [10] Return freeze x$7_@0:TObject scope0 [1:10]: - dependency: read a$6 - - dependency: mutate x$7_@0.z -scope1 [7:8]: - - dependency: mutate x$7_@0.z ``` ## Reactive Scopes @@ -50,14 +47,14 @@ scope1 [7:8]: function foo( a, ) { - scope @0 [1:10] deps=[read a$6, mutate x$7_@0.z] { + scope @0 [1:10] deps=[read a$6] { [1] Const mutate x$7_@0:TObject[1:10] = Object { } if (read a$6) { [3] Const mutate y$8_@0:TObject[1:10] = Object { } [4] Reassign mutate x$7_@0.y[1:10] = read y$8_@0:TObject [5] Call mutate mutate$4:TFunction(mutate y$8_@0:TObject) } else { - scope @1 [7:8] deps=[mutate x$7_@0.z] { + scope @1 [7:8] deps=[] { [7] Const mutate z$9_@1:TObject = Object { } } [8] Reassign mutate x$7_@0.z[1:10] = read z$9_@1:TObject diff --git a/compiler/forget/src/__tests__/fixtures/hir/switch-non-final-default.expect.md b/compiler/forget/src/__tests__/fixtures/hir/switch-non-final-default.expect.md index 16d2bdc5be..737625cfc0 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/switch-non-final-default.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/switch-non-final-default.expect.md @@ -61,8 +61,6 @@ bb1: scope2 [12:13]: - dependency: read Component$0 - dependency: freeze x$10_@1:TFunction - - dependency: read y$11_@1.push - - dependency: read props$9.p4 scope3 [14:15]: - dependency: read Component$0 - dependency: freeze y$11_@1:TPrimitive @@ -98,7 +96,7 @@ function Component( } } } - scope @2 [12:13] deps=[read Component$0, freeze x$10_@1:TFunction, read y$11_@1.push, read props$9.p4] { + scope @2 [12:13] deps=[read Component$0, freeze x$10_@1:TFunction] { [12] Const mutate child$19_@2 = JSX } [13] Call read y$11_@1.push(read props$9.p4) diff --git a/compiler/forget/src/__tests__/fixtures/hir/switch.expect.md b/compiler/forget/src/__tests__/fixtures/hir/switch.expect.md index a4eb3d3ea8..a37c1a59fc 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/switch.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/switch.expect.md @@ -56,8 +56,6 @@ bb1: scope3 [12:13]: - dependency: read Component$0 - dependency: freeze x$9_@1:TFunction - - dependency: read y$10_@1.push - - dependency: read props$8.p4 scope4 [14:15]: - dependency: read Component$0 - dependency: read y$10_@1:TPrimitive @@ -88,7 +86,7 @@ function Component( } } } - scope @3 [12:13] deps=[read Component$0, freeze x$9_@1:TFunction, read y$10_@1.push, read props$8.p4] { + scope @3 [12:13] deps=[read Component$0, freeze x$9_@1:TFunction] { [12] Const mutate child$19_@3 = JSX } [13] Call read y$10_@1.push(read props$8.p4) diff --git a/compiler/forget/src/__tests__/fixtures/hir/type-binary-operator.expect.md b/compiler/forget/src/__tests__/fixtures/hir/type-binary-operator.expect.md index d93e0f9cc1..23adc01b74 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/type-binary-operator.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/type-binary-operator.expect.md @@ -27,8 +27,7 @@ bb2: bb1: predecessor blocks: bb2 bb0 [7] Return -scope1 [2:3]: - - dependency: read a$7_@0:TPrimitive + ``` ## Reactive Scopes @@ -39,7 +38,7 @@ function component( scope @0 [1:2] deps=[] { [1] Const mutate a$7_@0:TPrimitive = Call mutate some$2:TFunction() } - scope @1 [2:3] deps=[read a$7_@0:TPrimitive] { + scope @1 [2:3] deps=[] { [2] Const mutate b$8_@1:TPrimitive = Call mutate someOther$4:TFunction() } [3] Const mutate $9:TPrimitive = Binary read a$7_@0:TPrimitive > read b$8_@1:TPrimitive diff --git a/compiler/forget/src/__tests__/fixtures/hir/type-test-field-store.expect.md b/compiler/forget/src/__tests__/fixtures/hir/type-test-field-store.expect.md index faa84d881a..d501e48768 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/type-test-field-store.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/type-test-field-store.expect.md @@ -20,10 +20,7 @@ bb0: [3] Reassign mutate x$4_@0.t[1:4] = read q$5_@1:TObject [4] Const mutate z$6:TObject = read x$4_@0.t [5] Return -scope0 [1:4]: - - dependency: mutate x$4_@0.t -scope1 [2:3]: - - dependency: mutate x$4_@0.t + ``` ## Reactive Scopes @@ -31,9 +28,9 @@ scope1 [2:3]: ``` function component( ) { - scope @0 [1:4] deps=[mutate x$4_@0.t] { + scope @0 [1:4] deps=[] { [1] Const mutate x$4_@0:TObject[1:4] = Object { } - scope @1 [2:3] deps=[mutate x$4_@0.t] { + scope @1 [2:3] deps=[] { [2] Const mutate q$5_@1:TObject = Object { } } [3] Reassign mutate x$4_@0.t[1:4] = read q$5_@1:TObject diff --git a/compiler/forget/src/__tests__/fixtures/hir/type-test-return-type-inference.expect.md b/compiler/forget/src/__tests__/fixtures/hir/type-test-return-type-inference.expect.md index e36dd715a1..225679a966 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/type-test-return-type-inference.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/type-test-return-type-inference.expect.md @@ -30,8 +30,7 @@ bb1: predecessor blocks: bb2 bb0 [7] Const mutate z$12_@3:TPrimitive = Call mutate foo$2:TFunction() [8] Return -scope1 [2:3]: - - dependency: read x$7_@0:TPrimitive + ``` ## Reactive Scopes @@ -42,7 +41,7 @@ function component( scope @0 [1:2] deps=[] { [1] Const mutate x$7_@0:TPrimitive = Call mutate foo$2:TFunction() } - scope @1 [2:3] deps=[read x$7_@0:TPrimitive] { + scope @1 [2:3] deps=[] { [2] Const mutate y$8_@1:TPrimitive = Call mutate foo$2:TFunction() } [3] Const mutate $9:TPrimitive = Binary read x$7_@0:TPrimitive > read y$8_@1:TPrimitive