Permalink
Browse files

Remove flow sensitive tracking of mutability

Summary:
Implementation of flow sensitive tracking of mutability is currently not working properly . It can be fixed by:
- removing flow sensitive aspect from mutability tracking and simply ensuring that everywhere mutability flavor for each local stays the same.
- pushing flow sensitive bits to use exising definite assignment checking logic.

Reviewed By: DavidSnider

Differential Revision: D12939757

fbshipit-source-id: bc573b4f88339bc22c80b95508a14ba5cb2899ea
  • Loading branch information...
vladima authored and hhvm-bot committed Nov 8, 2018
1 parent 14a9c21 commit b60cb3ef7d159253495dcac552d3565249fe1044
@@ -843,11 +843,14 @@ let rx_move_invalid_location pos =
add (Naming.err_code Naming.RxMoveInvalidLocation) pos
"Rx\\move is only allowed in argument position or as right hand side of the assignment."
let undefined pos var_name =
let undefined ~in_rx_scope pos var_name =
let rx_scope_clarification =
if in_rx_scope then "or unsets "
else "" in
add (Naming.err_code Naming.Undefined) pos ("Variable "^var_name^
" is undefined, "^
"or there exists at least one control flow path reaching this point which "^
"does not define "^var_name^".")
"does not define " ^ rx_scope_clarification ^ var_name ^".")
let this_reserved pos =
add (Naming.err_code Naming.ThisReserved) pos
@@ -81,7 +81,7 @@ module type S = sig
val error_class_attribute_already_bound : string -> string -> Pos.t -> Pos.t -> unit
val unbound_name : Pos.t -> string -> [< `cls | `func | `const ] -> unit
val different_scope : Pos.t -> string -> Pos.t -> unit
val undefined : Pos.t -> string -> unit
val undefined : in_rx_scope: bool -> Pos.t -> string -> unit
val this_reserved : Pos.t -> unit
val start_with_T : Pos.t -> unit
val already_bound : Pos.t -> string -> unit
@@ -444,7 +444,7 @@ end = struct
let found_dollardollar (_, lenv) p =
match !(lenv.pipe_locals) with
| [] ->
Errors.undefined p SN.SpecialIdents.dollardollar; (* TODO better error *)
Errors.undefined ~in_rx_scope:false p SN.SpecialIdents.dollardollar; (* TODO better error *)
Local_id.make_scoped SN.SpecialIdents.dollardollar
| pipe_scope :: scopes ->
let pipe_scope = { pipe_scope with used_dollardollar = true } in
@@ -920,7 +920,8 @@ let get_local_in_ctx env ?error_if_undef_at_pos:p x ctx =
let error_if_pos_provided posopt =
match posopt with
| Some p when not !Autocomplete.auto_complete ->
Errors.undefined p (LID.to_string x);
let in_rx_scope = env_local_reactive env in
Errors.undefined ~in_rx_scope p (LID.to_string x);
| _ -> () in
let lcl = LID.Map.get x ctx in
match lcl with
@@ -80,7 +80,7 @@ let handle_value_in_return
| Immutable -> "(non-mutable)"
| MaybeMutable -> "(maybe-mutable)"
| Borrowed -> "(borrowed)"
| MutableUnset | Mutable -> assert false in
| Mutable -> assert false in
Errors.invalid_mutable_return_result (T.get_position e) fun_pos kind in
let error_borrowed_as_immutable e =
(* attempt to return borrowed value as immutable *)
@@ -102,13 +102,13 @@ let handle_value_in_return
| T.Pipe (_, _, r) ->
(* ok for pipe if rhs returns mutable *)
aux r
| T.Lvar (p, id) ->
| T.Lvar (_, id) ->
let mut_env = Env.get_env_mutability env in
begin match LMap.get id mut_env with
| Some (_, Mutable) ->
| Some (p, Mutable) ->
(* it is ok to return mutably owned values *)
let env = Env.unset_local env id in
Env.env_with_mut env (LMap.add id (p, MutableUnset) mut_env)
Env.env_with_mut env (LMap.add id (p, Mutable) mut_env)
| Some (_, Borrowed) when not function_returns_mutable ->
(* attempt to return borrowed value as immutable
unless function is marked with __ReturnsVoidToRx in which case caller
@@ -197,12 +197,13 @@ let freeze_or_move_local
(invalid_use : Pos.t -> unit)
: Typing_env.env =
match tel with
| [_, T.Any] -> env
| [(_, T.Lvar (id_pos, id));] ->
let mut_env = Env.get_env_mutability env in
begin match LMap.get id mut_env with
| Some (_, Mutable) ->
| Some (p, Mutable) ->
let env = Env.unset_local env id in
Env.env_with_mut env (LMap.add id (id_pos, MutableUnset) mut_env)
Env.env_with_mut env (LMap.add id (p, Mutable) mut_env)
| Some x ->
invalid_target p id_pos (to_string x);
env
@@ -375,8 +376,9 @@ let enforce_mutable_constructor_call env ctor_fty el =
let enforce_mutable_call (env : Typing_env.env) (te : T.expr) =
match snd te with
| T.Call (_, (_, T.Id id), _, el, _)
| T.Call (_, (_, T.Fun_id id), _, el, _) ->
| T.Call (_, (_, T.Id (_, s as id)), _, el, _)
| T.Call (_, (_, T.Fun_id (_, s as id)), _, el, _)
when s <> SN.Rx.move && s <> SN.Rx.freeze ->
begin match Env.get_fun env (snd id) with
| Some fty ->
check_mutability_fun_params env Borrowable_args.empty fty el
@@ -12,13 +12,11 @@ open Core_kernel
See typing_mutability.ml for a description of the fields.
*)
module LMap = Local_id.Map
type mut_type =
Mutable | MutableUnset | Borrowed | MaybeMutable | Immutable
type mut_type = Mutable | Borrowed | MaybeMutable | Immutable
type mutability = Pos.t * mut_type
let to_string_ = function
| Mutable -> "mutably-owned"
| MutableUnset -> "mutably-owned (unset)"
| Borrowed -> "mutably-borrowed"
| MaybeMutable -> "maybe-mutable"
| Immutable -> "immutable"
@@ -46,35 +44,21 @@ let intersect_mutability
(m1 : mutability_env)
(m2 : mutability_env)
: mutability_env =
(* Check for any variables that were unset in one scope but not the other *)
LMap.iter
begin fun id _ ->
match LMap.get id m1, LMap.get id m2 with
| Some (_, Mutable), Some (p, MutableUnset)
| Some (p, MutableUnset), Some (_, Mutable) ->
Errors.inconsistent_unset p;
| _ -> ()
end parent_mut;
let merge ~keep_left _id v1_opt v2_opt =
let merge _id v1_opt v2_opt =
match v1_opt, v2_opt with
| Some (p1, mut1), Some (p2, mut2) ->
let assumed_mut =
(* do a conservative merge for mutability values *)
begin match mut1, mut2 with
| Mutable, MutableUnset | MutableUnset, Mutable -> MutableUnset
| _ ->
if mut1 = mut2 then mut1 else begin
Errors.inconsistent_mutability
p1 (to_string_ mut1)
(Some (p2, (to_string_ mut2)));
mut1
end
end in
if mut1 = mut2 then mut1 else begin
Errors.inconsistent_mutability
p1 (to_string_ mut1)
(Some (p2, (to_string_ mut2)));
mut1
end in
Some (p1, assumed_mut)
| Some v, None | None, Some v when keep_left -> Some v
| Some v, None | None, Some v -> Some v
| _ -> None in
(* intersect variables in child maps, keep only entries that exists in both *)
let acc = LMap.merge (merge ~keep_left:false) m1 m2 in
(* combine result with parent env preserving items from parent *)
let acc = LMap.merge (merge ~keep_left:true) parent_mut acc in
(* ensure not conflicting mutability flavors in child scopes *)
let acc = LMap.merge merge m1 m2 in
(* ensure no conflicting flavors in child and parent scopes *)
let acc = LMap.merge merge parent_mut acc in
acc
@@ -21,7 +21,7 @@
reassigned. They are essentially read only.
*)
type mut_type =
Mutable | MutableUnset | Borrowed | MaybeMutable | Immutable
Mutable | Borrowed | MaybeMutable | Immutable
type mutability =
Pos.t * mut_type
@@ -0,0 +1,15 @@
<?hh
class Foo {}
<<__RxShallow>>
function some_func(<<__OwnedMutable>>Foo $foo): void {
// OK
if (Rx\IS_ENABLED) {
some_other_func(HH\Rx\move($foo));
} else {
}
}
<<__Rx>>
function some_other_func(<<__OwnedMutable>>Foo $foo): void {}
@@ -0,0 +1 @@
No errors
@@ -0,0 +1,14 @@
<?hh
class Foo {}
<<__RxShallow>>
function some_third_func(<<__OwnedMutable>>Foo $foo, bool $cond): void {
// OK
if ($cond) {
some_other_func(HH\Rx\move($foo));
} else {
}
}
<<__Rx>>
function some_other_func(<<__OwnedMutable>>Foo $foo): void {}
@@ -0,0 +1 @@
No errors
@@ -0,0 +1,15 @@
<?hh
class Foo {}
<<__RxShallow>>
function some_third_func(<<__OwnedMutable>>Foo $foo, bool $cond): void {
if ($cond) {
some_other_func(HH\Rx\move($foo));
} else {
}
// ERROR: $foo might be unset
$foo;
}
<<__Rx>>
function some_other_func(<<__OwnedMutable>>Foo $foo): void {}
@@ -0,0 +1,2 @@
File "cond_rx3.php", line 11, characters 5-8:
Variable $foo is undefined, or there exists at least one control flow path reaching this point which does not define or unsets $foo. (Naming[2050])
@@ -0,0 +1,28 @@
<?hh // strict
class Foo {}
<<__Rx>>
function some_function(<<__OwnedMutable>>Foo $foo, bool $c1, bool $c2): int {
if ($c1) {
return some_other_function(\HH\Rx\freeze($foo));
}
if ($c2) {
try {
return g();
} catch (Exception $ex) {
return some_other_function(\HH\Rx\freeze($foo));
}
}
return 4;
}
<<__Rx>>
function g(): int {
return 42;
}
<<__Rx>>
function some_other_function(Foo $foo): int {
return 7;
}
@@ -0,0 +1 @@
No errors
@@ -1,2 +1,2 @@
File "mutability_flavors2_1.php", line 14, characters 5-6:
Variable $a is undefined, or there exists at least one control flow path reaching this point which does not define $a. (Naming[2050])
Variable $a is undefined, or there exists at least one control flow path reaching this point which does not define or unsets $a. (Naming[2050])
@@ -1,2 +1,2 @@
File "mutability_flavors3.php", line 9, characters 3-4:
Cannot assign immutable value to mutably-owned (unset) local variable. Mutability flavor of local variable cannot be altered. (Typing[4294])
Cannot assign immutable value to mutably-owned local variable. Mutability flavor of local variable cannot be altered. (Typing[4294])
@@ -1,4 +1,4 @@
File "mutability_flavors7.php", line 9, characters 5-6:
Inconsistent mutability of local variable, here local is immutable (Typing[4293])
File "mutability_flavors7.php", line 14, characters 24-25:
But here it is mutably-owned (unset)
File "mutability_flavors7.php", line 13, characters 5-6:
But here it is mutably-owned
@@ -1,2 +1,2 @@
File "rx_move4.php", line 11, characters 24-25:
Variable $a is undefined, or there exists at least one control flow path reaching this point which does not define $a. (Naming[2050])
Variable $a is undefined, or there exists at least one control flow path reaching this point which does not define or unsets $a. (Naming[2050])
@@ -1,2 +1,2 @@
File "rx_move5.php", line 12, characters 11-12:
Variable $a is undefined, or there exists at least one control flow path reaching this point which does not define $a. (Naming[2050])
Variable $a is undefined, or there exists at least one control flow path reaching this point which does not define or unsets $a. (Naming[2050])
@@ -21,6 +21,7 @@ function basic(bool $b): void {
} else {
}
$y;
// invalid, $y is frozen in one scope but not another
}
@@ -1,2 +1,2 @@
File "test_freeze2.php", line 20, characters 25-26:
This variable is unset (via Rx\freeze or Rx\move) in one scope but not the other (Typing[4209])
File "test_freeze2.php", line 24, characters 3-4:
Variable $y is undefined, or there exists at least one control flow path reaching this point which does not define or unsets $y. (Naming[2050])

0 comments on commit b60cb3e

Please sign in to comment.