[name_resolver] Avoiding creating phi nodes when merging with havoc if the variable is initialized

Summary:
Currently, we only have this behavior when the variable declaration is annotated, but we still create phi nodes if the variable is initialized. There is no good reason for such split, except that we want to maintain the behavior with the inference before new environment.

However, this decision can cause serious perf issues under LTI, when the union type won't just merge together in a single type, but remained to be unions. Consider the following example:

```
let data1: Array<string> = [];
let data = data1
if (true) data = data.filter(s => s.substring(0));
if (true) data = data.filter(s => s.substring(0));
// repeat n times
if (true) data = data.filter(s => s.substring(0));
(data: Array<string>);
```

At the point of final cast, the name_resolver will tell us the read of `data` is phi node, and the type will be obtained by flowing n types into a result tvar. Pre-LTI, these n types will coalesce into one type, so the perf is not terrible, but under LTI, the result will be a size-n union type. The issue also exists for each if branch, where the size of union type is linearly increasing. Under LTI, we will also do contextual typing for `s`, so we will do work in each item in the if-chain proportional to the size of the union type, which leads to a quadratic blowup.

To fix it, I extend the avoiding phi-node logic to include initialized variables as well. This also fits in the LTI story, since under LTI every expressions, like annotations, cannot have their type widened.

Changelog: [errors] When a variable is initialized at declaration site, subsequent assignment in a conditional branch will no longer cause later reads to get a union type. This is already the case if the variable is annotated. As a result of this change, you might get new type errors. Example: https://flow.org/try/#0DYUwLgBAJghmMQLwQEQEYUG4BQBLAZhABRgBOAriAJTRwLJo6zyYQD0bEADqSAG64A9uQDOwAJ4RckAO65gwCAAsYfEBDDiu6gHbkAtgCMQpCAB8IIsrh0BzADQQdgmVNnzFKtRq3qrpG1tsIA

Reviewed By: panagosg7

Differential Revision:
D43134095 (d4f94ee)

------------------------------------------------------------------------
(from ad59e13768a9567e487b642b9a54ac6b6b34027e)

fbshipit-source-id: 48f15ff0e74cd98880e89160b918212a5f419749

Author: SamChou19815

src/analysis/env_builder/__tests__/env_builder_refinement_test.ml

@@ -1537,9 +1537,7 @@ try {
           Global invariant
         };
         (5, 2) to (5, 3) => {
-          (1, 4) to (1, 5): (`x`),
-          (3, 24) to (3, 25): (`x`),
-          {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`x`)}
+          (1, 4) to (1, 5): (`x`)
         }] |}]
 
 let%expect_test "nested_logical_throw_and" =
@@ -1664,8 +1662,7 @@ x;|};
     [%expect {|
       [
         (5, 0) to (5, 1) => {
-          (1, 4) to (1, 5): (`x`),
-          (3, 2) to (3, 3): (`x`)
+          (1, 4) to (1, 5): (`x`)
         }] |}]
 
 let%expect_test "if_no_else_statement_with_assignment_simple_with_annotation" =
@@ -1937,13 +1934,10 @@ x;|};
           {refinement = Not (Maybe); writes = (2, 4) to (2, 5): (`y`)}
         };
         (9, 0) to (9, 1) => {
-          (2, 4) to (2, 5): (`y`),
-          {refinement = Not (Maybe); writes = (2, 4) to (2, 5): (`y`)},
-          {refinement = Not (Not (Maybe)); writes = (2, 4) to (2, 5): (`y`)}
+          (2, 4) to (2, 5): (`y`)
         };
         (10, 0) to (10, 1) => {
-          (1, 4) to (1, 5): (`x`),
-          {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`x`)}
+          (1, 4) to (1, 5): (`x`)
         }] |}]
 
 let%expect_test "while_with_runtime_writes" =
@@ -1978,7 +1972,7 @@ x;|};
           (2, 4) to (2, 5): (`y`)
         };
         (10, 0) to (10, 1) => {
-          {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`x`),(5, 4) to (5, 5): (`x`)}
+          {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`x`)}
         }] |}]
 
 let%expect_test "while_with_var_write" =
@@ -2042,9 +2036,7 @@ x;|};
           {refinement = Not (Maybe); writes = (2, 4) to (2, 5): (`y`)}
         };
         (9, 0) to (9, 1) => {
-          (2, 4) to (2, 5): (`y`),
-          {refinement = Not (Maybe); writes = (2, 4) to (2, 5): (`y`)},
-          {refinement = Not (Not (Maybe)); writes = (2, 4) to (2, 5): (`y`)}
+          (2, 4) to (2, 5): (`y`)
         };
         (10, 0) to (10, 1) => {
           {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`x`)}
@@ -2074,7 +2066,7 @@ x;|};
           {refinement = Not (3); writes = {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`x`)}}
         };
         (8, 0) to (8, 1) => {
-          {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`x`),{refinement = 3; writes = (1, 4) to (1, 5): (`x`)},{refinement = Not (3); writes = (1, 4) to (1, 5): (`x`)}}
+          {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`x`)}
         }] |}]
 
 let%expect_test "do_while" =
@@ -2160,14 +2152,13 @@ x;|};
           (2, 4) to (2, 5): (`y`)
         };
         (8, 9) to (8, 10) => {
-          (1, 4) to (1, 5): (`x`),
-          (5, 4) to (5, 5): (`x`)
+          (1, 4) to (1, 5): (`x`)
         };
         (9, 0) to (9, 1) => {
           (2, 4) to (2, 5): (`y`)
         };
         (10, 0) to (10, 1) => {
-          {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`x`),(5, 4) to (5, 5): (`x`)}
+          {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`x`)}
         }] |}]
 
 let%expect_test "do_while_continue" =
@@ -2320,13 +2311,10 @@ x;|};
           {refinement = Not (Maybe); writes = (2, 4) to (2, 5): (`y`)}
         };
         (9, 0) to (9, 1) => {
-          (2, 4) to (2, 5): (`y`),
-          {refinement = Not (Maybe); writes = (2, 4) to (2, 5): (`y`)},
-          {refinement = Not (Not (Maybe)); writes = (2, 4) to (2, 5): (`y`)}
+          (2, 4) to (2, 5): (`y`)
         };
         (10, 0) to (10, 1) => {
-          (1, 4) to (1, 5): (`x`),
-          {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`x`)}
+          (1, 4) to (1, 5): (`x`)
         }] |}]
 
 let%expect_test "for_no_init_no_update_with_runtime_writes" =
@@ -2361,7 +2349,7 @@ x;|};
           (2, 4) to (2, 5): (`y`)
         };
         (10, 0) to (10, 1) => {
-          {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`x`),(5, 4) to (5, 5): (`x`)}
+          {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`x`)}
         }] |}]
 
 let%expect_test "for_no_init_no_update_continue" =
@@ -2411,9 +2399,7 @@ x;|};
           {refinement = Not (Maybe); writes = (2, 4) to (2, 5): (`y`)}
         };
         (9, 0) to (9, 1) => {
-          (2, 4) to (2, 5): (`y`),
-          {refinement = Not (Maybe); writes = (2, 4) to (2, 5): (`y`)},
-          {refinement = Not (Not (Maybe)); writes = (2, 4) to (2, 5): (`y`)}
+          (2, 4) to (2, 5): (`y`)
         };
         (10, 0) to (10, 1) => {
           {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`x`)}
@@ -2443,9 +2429,7 @@ x;|};
           {refinement = Not (3); writes = {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`x`)}}
         };
         (8, 0) to (8, 1) => {
-          (1, 4) to (1, 5): (`x`),
-          {refinement = 3; writes = {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`x`)}},
-          {refinement = Not (3); writes = (1, 4) to (1, 5): (`x`)}
+          (1, 4) to (1, 5): (`x`)
         }] |}]
 
 let%expect_test "for_shadow" =
@@ -2524,9 +2508,7 @@ y;|};
           {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`y`)}
         };
         (8, 0) to (8, 1) => {
-          (1, 4) to (1, 5): (`y`),
-          {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`y`)},
-          {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`y`)}
+          (1, 4) to (1, 5): (`y`)
         }] |}]
 
 let%expect_test "for_with_runtime_writes" =
@@ -2604,9 +2586,7 @@ y;|};
           {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`y`)}
         };
         (8, 0) to (8, 1) => {
-          (1, 4) to (1, 5): (`y`),
-          {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`y`)},
-          {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`y`)}
+          (1, 4) to (1, 5): (`y`)
         }] |}]
 
 let%expect_test "no_havoc_before_write_seen" =
@@ -2846,9 +2826,7 @@ y;|};
           {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`y`)}
         };
         (8, 0) to (8, 1) => {
-          (1, 4) to (1, 5): (`y`),
-          {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`y`)},
-          {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`y`)}
+          (1, 4) to (1, 5): (`y`)
         }] |}]
 
 let%expect_test "for_in_with_runtime_writes" =
@@ -2880,8 +2858,7 @@ x;|};
           (2, 4) to (2, 5): (`y`)
         };
         (10, 0) to (10, 1) => {
-          (1, 4) to (1, 5): (`x`),
-          (5, 4) to (5, 5): (`x`)
+          (1, 4) to (1, 5): (`x`)
         }] |}]
 
 let%expect_test "for_in_continue" =
@@ -2920,9 +2897,7 @@ y;|};
           {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`y`)}
         };
         (8, 0) to (8, 1) => {
-          (1, 4) to (1, 5): (`y`),
-          {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`y`)},
-          {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`y`)}
+          (1, 4) to (1, 5): (`y`)
         }] |}]
 
 let%expect_test "for_in_reassign_right" =
@@ -2937,8 +2912,7 @@ stuff;|};
           (1, 4) to (1, 9): (`stuff`)
         };
         (5, 0) to (5, 5) => {
-          (1, 4) to (1, 9): (`stuff`),
-          (3, 2) to (3, 7): (`stuff`)
+          (1, 4) to (1, 9): (`stuff`)
         }] |}]
 
 let%expect_test "for_of" =
@@ -3057,9 +3031,7 @@ y;|};
           {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`y`)}
         };
         (8, 0) to (8, 1) => {
-          (1, 4) to (1, 5): (`y`),
-          {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`y`)},
-          {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`y`)}
+          (1, 4) to (1, 5): (`y`)
         }] |}]
 
 let%expect_test "for_of_with_runtime_writes" =
@@ -3091,8 +3063,7 @@ x;|};
           (2, 4) to (2, 5): (`y`)
         };
         (10, 0) to (10, 1) => {
-          (1, 4) to (1, 5): (`x`),
-          (5, 4) to (5, 5): (`x`)
+          (1, 4) to (1, 5): (`x`)
         }] |}]
 
 let%expect_test "for_of_continue" =
@@ -3131,9 +3102,7 @@ y;|};
           {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`y`)}
         };
         (8, 0) to (8, 1) => {
-          (1, 4) to (1, 5): (`y`),
-          {refinement = Not (Maybe); writes = (1, 4) to (1, 5): (`y`)},
-          {refinement = Not (Not (Maybe)); writes = (1, 4) to (1, 5): (`y`)}
+          (1, 4) to (1, 5): (`y`)
         }] |}]
 
 let%expect_test "for_of_reassign_right" =
@@ -3148,8 +3117,7 @@ stuff;|};
           (1, 4) to (1, 9): (`stuff`)
         };
         (5, 0) to (5, 5) => {
-          (1, 4) to (1, 9): (`stuff`),
-          (3, 2) to (3, 7): (`stuff`)
+          (1, 4) to (1, 9): (`stuff`)
         }] |}]
 
 let%expect_test "invariant" =
@@ -3254,8 +3222,7 @@ try {
           Global invariant
         };
         (8, 2) to (8, 3) => {
-          (1, 4) to (1, 5): (`x`),
-          (5, 21) to (5, 22): (`x`)
+          (1, 4) to (1, 5): (`x`)
         }] |}]
 
 let%expect_test "switch_empty" =
@@ -4149,8 +4116,7 @@ x.foo; // No refinement
           (3, 4) to (3, 5): (`x`)
         };
         (8, 0) to (8, 1) => {
-          (3, 4) to (3, 5): (`x`),
-          {refinement = SentinelR foo; writes = (3, 4) to (3, 5): (`x`)}
+          (3, 4) to (3, 5): (`x`)
         }] |}]
 
 let%expect_test "heap_refinement_while_loop_subject_changed" =
@@ -4175,8 +4141,7 @@ x.foo; // No heap refinement here
           {refinement = 3; writes = projection at (3, 7) to (3, 12)}
         };
         (8, 0) to (8, 1) => {
-          (2, 4) to (2, 5): (`x`),
-          (5, 2) to (5, 3): (`x`)
+          (2, 4) to (2, 5): (`x`)
         }] |}]
 
 let%expect_test "heap_refinement_while_loop_projection_changed" =
@@ -4211,8 +4176,7 @@ x.foo;
           {refinement = 4; writes = {refinement = 3; writes = projection at (4, 7) to (4, 12)}}
         };
         (9, 0) to (9, 1) => {
-          (3, 4) to (3, 5): (`x`),
-          {refinement = SentinelR foo; writes = {refinement = SentinelR foo; writes = (3, 4) to (3, 5): (`x`)}}
+          (3, 4) to (3, 5): (`x`)
         }] |}]
 
 let%expect_test "heap_refinement_while_loop_negated" =
@@ -4276,7 +4240,7 @@ x.foo;
           {refinement = 4; writes = {refinement = 3; writes = projection at (3, 7) to (3, 12)}}
         };
         (9, 0) to (9, 1) => {
-          {refinement = Not (SentinelR foo); writes = (2, 4) to (2, 5): (`x`),{refinement = SentinelR foo; writes = (2, 4) to (2, 5): (`x`)}}
+          {refinement = Not (SentinelR foo); writes = (2, 4) to (2, 5): (`x`)}
         };
         (9, 0) to (9, 5) => {
           {refinement = Not (3); writes = projection at (3, 7) to (3, 12),{refinement = 4; writes = projection at (3, 7) to (3, 12)}}

src/analysis/env_builder/name_resolver.ml

@@ -1321,10 +1321,23 @@ module Make
         match def_loc with
         | Some loc ->
           (match Env_api.Provider_api.providers_of_def provider_info loc with
-          (* We only use havoc as the merge if the providers that generate the havoc val are
-             annotated. Only when the havoc val is annotated, we know for certain that it wouldn't
-             be widened in the branches we are merging together. *)
-          | Some { Env_api.Provider_api.state = Find_providers.AnnotatedVar _; _ }
+          (* We only use havoc as the merge if the variable's provider is at the declaration site.
+             i.e. annotated or initialized at declaration. This prevents us from creating large
+             union types in later if chains.
+
+             e.g.
+             let x = init; // or let x: annot = ...;
+             if (...) x = expr_1;
+             if (...) x = expr_2;
+             // ...
+             if (...) x = expr_n;
+             x should still has type of init, instead of a size-n+1 union type. *)
+          | Some
+              {
+                Env_api.Provider_api.state =
+                  Find_providers.AnnotatedVar _ | Find_providers.InitializedVar;
+                _;
+              }
             when can_merge_with_havoc v1 v2 || can_merge_with_havoc v2 v1 ->
             havoc
           | _ -> Val.merge v1 v2)

tests/badly_positioned_flow_use_op/b.js

@@ -7,17 +7,14 @@ declare function foo(data: Data): void;
 const o = {
   fun: foo,
 }
-/*
-The error position for this one is bad,
-as it points to line 5.
-*/
+
 function test1(b: boolean) {
   var data = { x: 0 };
   if (b) data = { x: 0, z: 0 };
-  o['fun'](data);
+  o['fun'](data); // no error
 }
 /*The error position for this one is ok.*/
 function test2(b: boolean) {
   var data = { z: 0 };
-  o['fun'](data);
+  o['fun'](data); // Error: { z: 0 } ~> Data
 }