Skip to content

Commit

Permalink
model x.foo++ with a new Number Val.write_state
Browse files Browse the repository at this point in the history
Summary:
This diff fixes duplicate errors we see on things like:

```
//flow
type O = {foo?: number};

declare var o: O;
o.foo++;
```
as mentioned in the previous diff.

These duplicate errors were caused because we were unifying the read of o.foo with the write of o.foo. `o.foo++` always creates a number, so this effectively unified `void | number` with `number`. The reason we unified the reads and writes is because we set the write of `o.foo++` in the env builder at the location of `o.foo`.

Since these operations always produce a number, there is no real reason to record a location and use the env to look it up. Instead we, introduce a new Val.write_state that represents a number. This allows us to avoid the clashing key issue entirely.

Changelog: [internal]

Reviewed By: SamChou19815

Differential Revision: D35200715

fbshipit-source-id: 97557c2ab6e0a981a6012392ea673a112ec7ab80
  • Loading branch information
jbrown215 authored and facebook-github-bot committed Mar 29, 2022
1 parent 0fbdcc1 commit 25e9495
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ let print_values refinement_of_id =
| Global name -> "Global " ^ name
| Unreachable _ -> "unreachable"
| Undefined _ -> "undefined"
| Number _ -> "number"
| DeclaredFunction l -> Printf.sprintf "declared function %s" (L.debug_to_string l)
in
fun values ->
Expand Down Expand Up @@ -4472,13 +4473,13 @@ x.a;
(2, 4) to (2, 5): (`x`)
};
(4, 0) to (4, 3) => {
(3, 2) to (3, 5): (some property)
number
};
(5, 0) to (5, 1) => {
(2, 4) to (2, 5): (`x`)
};
(5, 0) to (5, 3) => {
(4, 0) to (4, 3): (some property)
number
}] |}]

let%expect_test "op_assign_heap" =
Expand Down
4 changes: 4 additions & 0 deletions src/analysis/env_builder/env_api.ml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ module type S = sig
| Projection of L.t
| Unreachable of L.t
| Undefined of L.t Reason.virtual_reason
| Number of L.t Reason.virtual_reason
| DeclaredFunction of L.t

and write_locs = write_loc list
Expand Down Expand Up @@ -195,6 +196,7 @@ module Make
| Projection of L.t
| Unreachable of L.t
| Undefined of L.t Reason.virtual_reason
| Number of L.t Reason.virtual_reason
| DeclaredFunction of L.t

and write_locs = write_loc list
Expand Down Expand Up @@ -315,6 +317,7 @@ module Make
| Projection _ -> []
| Unreachable _ -> []
| Undefined r -> [Reason.poly_loc_of_reason r]
| Number r -> [Reason.poly_loc_of_reason r]
| DeclaredFunction l -> [l]

let rec refinements_of_write_loc ({ refinement_of_id; _ } as env) write_loc =
Expand Down Expand Up @@ -365,6 +368,7 @@ module Make
| Projection l -> Printf.sprintf "projection at %s" (L.debug_to_string l)
| Unreachable _ -> "unreachable"
| Undefined _ -> "undefined"
| Number _ -> "number"
| DeclaredFunction l -> Printf.sprintf "declared function %s" (L.debug_to_string l)
| Write reason ->
let loc = Reason.poly_loc_of_reason reason in
Expand Down
23 changes: 21 additions & 2 deletions src/analysis/env_builder/name_resolver.ml
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ module Make

val undefined : L.t virtual_reason -> t

val number : L.t virtual_reason -> t

val undeclared_class : L.t virtual_reason -> string -> t

val merge : t -> t -> t
Expand Down Expand Up @@ -227,6 +229,7 @@ module Make
val_t: t;
}
| Undefined of ALoc.t virtual_reason
| Number of ALoc.t virtual_reason
| DeclaredFunction of ALoc.t

and t = {
Expand Down Expand Up @@ -274,6 +277,8 @@ module Make

let undefined r = mk_with_write_state (Undefined r)

let number r = mk_with_write_state (Number r)

let undeclared_class def name = mk_with_write_state (UndeclaredClass { def; name })

let projection loc = mk_with_write_state @@ Projection loc
Expand Down Expand Up @@ -323,6 +328,7 @@ module Make
match t with
| Uninitialized _
| Undefined _
| Number _
| DeclaredFunction _
| Undeclared _
| DeclaredButSkipped _
Expand Down Expand Up @@ -360,6 +366,7 @@ module Make
match t with
| Uninitialized _
| Undefined _
| Number _
| DeclaredFunction _
| Undeclared _
| DeclaredButSkipped _
Expand Down Expand Up @@ -396,6 +403,7 @@ module Make
| Uninitialized l when WriteSet.cardinal vals <= 1 ->
Env_api.Uninitialized (mk_reason RUninitialized l)
| Undefined r -> Env_api.Undefined r
| Number r -> Env_api.Number r
| DeclaredFunction l -> Env_api.DeclaredFunction l
| Undeclared (name, loc)
| DeclaredButSkipped (name, loc) ->
Expand Down Expand Up @@ -431,6 +439,7 @@ module Make
| Undeclared _ -> []
| DeclaredButSkipped _ -> []
| Undefined _ -> []
| Number _ -> []
| DeclaredFunction _ -> []
| Uninitialized _ -> [v]
| UndeclaredClass _ -> [v]
Expand Down Expand Up @@ -1677,8 +1686,18 @@ module Make
ignore @@ this#expression argument;
ignore @@ this#pattern_expression argument;
let val_reason = mk_reason RSomeProperty loc in
let assigned_val = Val.one val_reason in
this#assign_member ~update_entry:true member loc assigned_val val_reason
let assigned_val = Val.number val_reason in
(* We explicitly write a number instead of using the location of the write to avoid
* location clashes between the read of the member and the write of the member. If
* we don't do this then expressions like o.x++ will attempt to unify the read of o.x
* with the write of o.x that happens in that update expression *)
(match RefinementKey.lookup_of_member member ~allow_optional:false with
| Some lookup ->
this#map_val_with_lookup
lookup
(fun _ -> assigned_val)
~create_val_for_heap:(Some (fun () -> assigned_val))
| _ -> ())
| _ -> (* given 'o()++`, read o *) ignore @@ this#expression argument
end;
expr
Expand Down
2 changes: 2 additions & 0 deletions src/typing/new_env.ml
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ module New_env = struct
| (Env_api.Undefined reason, _)
| (Env_api.Uninitialized reason, _) ->
Type.(VoidT.make reason |> with_trust Trust.bogus_trust)
| (Env_api.Number reason, _) -> Type.(NumT.make reason |> with_trust Trust.bogus_trust)
| (Env_api.DeclaredFunction loc, _) -> provider_type_for_def_loc ~intersect:true env loc
| (Env_api.Undeclared (_name, def_loc), (ForType | ForTypeof)) ->
Base.Option.value_exn (Loc_env.find_write env def_loc)
Expand Down Expand Up @@ -353,6 +354,7 @@ module New_env = struct
Base.List.exists
~f:(function
| Env_api.With_ALoc.Undefined _ -> true
| Env_api.With_ALoc.Number _ -> true
| Env_api.With_ALoc.DeclaredFunction _ -> true
| Env_api.With_ALoc.Uninitialized _ -> true
| Env_api.With_ALoc.UndeclaredClass _ -> true
Expand Down
1 change: 0 additions & 1 deletion tests/optional/.testconfig

This file was deleted.

0 comments on commit 25e9495

Please sign in to comment.