Skip to content

Commit

Permalink
[flow][tests] Do not aggressively invalidate refinements in loops for…
Browse files Browse the repository at this point in the history
… const-like variables

Summary:
This diff fixed the issue shown in the previous diff. The problem is that we see the Val has changed, because inside the loop we have a latent refinement (from `if (f(v)`) being applied to it.

While in general it seems to hard to avoid these bad interactions, I think we can at least avoid it for const-like variables. For const-like variables, we should be able to always keep the outer refinements.

Changelog: [fix] Flow will no longer invalidate refinements made before the loop for const-like variables within a loop ([example](https://flow.org/try/#1N4Igxg9gdgZglgcxALlAIwIZoKYBsD6uEEAztvhgE6UYCe+JADpdhgCYowa5kA0I2KAFcAtiRQAXSkOz9sADwxgJ+NPTbYuQ3BMnTZA+Y2yU4IwRO4A6SFBIrGVDGM7c+h46fNRLuKxJIGWh8MeT0ZfhYlCStpHzNsFBAMIQkIEQwJODAQfiEyfBE4eWw2fDgofDBMsAALfAA3KjgsXGxxZC4eAw0G-GhcWn9aY3wWZldu-g1mbGqJUoBaCRHEzrcDEgBrbAk62kXhXFxJ923d-cPRHEpTgyEoMDaqZdW7vKgoOfaSKgOKpqmDA+d4gB5fMA-P6LCCMLLQbiLOoYCqgh6-GDYRYIXYLSgkRZkCR4jpddwPfJLZjpOBkO4AX34kA0SQ0Tyo2AABDAHso4NBuQAKBrITn2UxQBAASlFaGIzygAG4ADpQVU8x7wqCchb2YWi8UVBCcgA+nKOuClnOAqs5nLgME5ws5AF4XebtJbrba7ZyWBIhJQlT76T6AO61OBtJ1SGRWm3a30Op0wYVS+OcgD0mc5swa-Pyg05JkoEEovHNEDDnIgW1FUGgV2Ofs0FWw3gk9pInJ2cJ9vpbAaDKsTdtDifH49yIAaJhI-KgSQy9hMIHpQA))

Reviewed By: jbrown215

Differential Revision: D68224844

fbshipit-source-id: 27db5bc4555ab4ce1d120997e8764f5d53b04a78
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Jan 15, 2025
1 parent 05aba2a commit 8d3d596
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 11 deletions.
19 changes: 17 additions & 2 deletions src/analysis/env_builder/name_resolver.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3382,7 +3382,7 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
let {
PartialEnvSnapshot.env_val = post_env_val;
heap_refinements = post_env_heap_refinements;
def_loc = _;
def_loc;
} =
match post_env_v with
| Some v -> v
Expand Down Expand Up @@ -3418,7 +3418,22 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
post_env_heap_refinements
acc
in
if Val.id_of_val pre_env_val = Val.id_of_val post_env_val then
let is_definitely_const_like =
match def_loc with
| None -> false
| Some loc ->
not
(Invalidation_api.should_invalidate
~all:true
invalidation_caches
prepass_info
prepass_values
loc
)
in
if
is_definitely_const_like || Val.id_of_val pre_env_val = Val.id_of_val post_env_val
then
(acc, None)
else
(RefinementKey.lookup_of_name name :: acc, None)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
return;
}
while (true) {
if (f(v)) { // spurious error
if (f(v)) { // ok: non-null refinement is kept
return;
}
}
Expand Down
13 changes: 5 additions & 8 deletions tests/refinements/refinements.exp
Original file line number Diff line number Diff line change
Expand Up @@ -7701,19 +7701,16 @@ References:

Error -------------------------------------------------------------- loop_incorrect_invalidation_regression_test.js:9:13

Cannot call `f` with `v` bound to `v` because null [1] is incompatible with string [2]. [incompatible-call]
Refined at [1]

loop_incorrect_invalidation_regression_test.js:9:13
9| if (f(v)) { // spurious error
9| if (f(v)) { // ok: non-null refinement is kept
^

References:
loop_incorrect_invalidation_regression_test.js:4:29
4| function test(v: string | null) {
^^^^ [1]
loop_incorrect_invalidation_regression_test.js:2:25
2| declare function f(v: string): boolean;
^^^^^^ [2]
loop_incorrect_invalidation_regression_test.js:5:9
5| if (v == null) {
^^^^^^^^^ [1]


Error --------------------------------------------------------------------------------------------- maybe_react.js:18:19
Expand Down

0 comments on commit 8d3d596

Please sign in to comment.