Skip to content

Commit 0a01c6d

Browse files
mrkmndzfacebook-github-bot
mrkmndz
authored and
facebook-github-bot
committed
Refactor variable access/typeorder code
Summary: There was a lot of repetition involved here, and adding IntVar in the follow up diff exposed this code smell. This also suggests we may need Type.Variable to make these many variable methods more explicit. Reviewed By: dkgi Differential Revision: D14736274 fbshipit-source-id: ee265b73ac469598e7200eedd2ec80536cdf5353
1 parent 4f9b6af commit 0a01c6d

File tree

5 files changed

+54
-59
lines changed

5 files changed

+54
-59
lines changed

‎analysis/test/typeOrderTest.ml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -836,6 +836,7 @@ let test_less_or_equal _ =
836836
insert order Type.Bottom;
837837
insert order Type.Any;
838838
insert order Type.Top;
839+
insert order Type.object_primitive;
839840
add_simple (Type.variable "_1");
840841
add_simple (Type.variable "_2");
841842
add_simple (Type.variable "_T");
@@ -1881,6 +1882,7 @@ let test_join _ =
18811882
insert order Type.Bottom;
18821883
insert order Type.Any;
18831884
insert order Type.Top;
1885+
add_simple Type.object_primitive;
18841886
add_simple (Type.variable "_1");
18851887
add_simple (Type.variable "_2");
18861888
add_simple (Type.variable "_T");
@@ -1897,7 +1899,7 @@ let test_join _ =
18971899
18981900
connect order ~predecessor:Type.Bottom ~successor:Type.integer;
18991901
connect order ~predecessor:Type.integer ~successor:Type.float;
1900-
connect order ~predecessor:Type.float ~successor:Type.Top;
1902+
connect order ~predecessor:Type.float ~successor:Type.object_primitive;
19011903
19021904
connect order ~predecessor:Type.Bottom ~successor:!"A";
19031905
@@ -2132,7 +2134,7 @@ let test_join _ =
21322134
(* Variables. *)
21332135
assert_type_equal
21342136
(join order Type.integer (Type.variable "T"))
2135-
(Type.union [Type.integer; Type.variable "T"]);
2137+
Type.object_primitive;
21362138
assert_type_equal
21372139
(join order Type.integer (Type.variable ~constraints:(Type.Bound Type.string) "T"))
21382140
(Type.union [Type.string; Type.integer]);

‎analysis/type.ml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2261,6 +2261,16 @@ let mark_free_variables_as_escaped ?specific annotation =
22612261
instantiate annotation ~constraints:(Map.find constraints)
22622262

22632263

2264+
let upper_bound { constraints; _ } =
2265+
match constraints with
2266+
| Unconstrained ->
2267+
object_primitive
2268+
| Bound bound ->
2269+
bound
2270+
| Explicit explicits ->
2271+
union explicits
2272+
2273+
22642274
let contains_escaped_free_variable =
22652275
exists ~predicate:is_escaped_free_variable
22662276

‎analysis/type.mli

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,7 @@ val free_simulated_bound_variables: t -> t
294294
val is_resolved: t -> bool
295295
val instantiate_free_variables: replacement:t -> t -> t
296296
val mark_free_variables_as_escaped: ?specific: variable list -> t -> t
297+
val upper_bound: variable -> t
297298

298299
val is_escaped_free_variable: t -> bool
299300
val contains_escaped_free_variable: t -> bool

‎analysis/typeCheck.ml

Lines changed: 30 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -985,37 +985,36 @@ module State = struct
985985
(* Attribute access. *)
986986
let rec extract_access_data annotation ~inside_meta =
987987
let reset_instantiated data = { data with instantiated = annotation } in
988-
match annotation with
989-
| Type.Top | Type.Bottom | Type.Any ->
990-
[]
991-
| annotation when Type.equal annotation Type.ellipsis ->
992-
[]
993-
| Type.Union annotations ->
994-
List.concat_map annotations ~f:(extract_access_data ~inside_meta)
995-
| Type.Variable { constraints = Unconstrained; _ } ->
996-
extract_access_data Type.object_primitive ~inside_meta
997-
|> List.map ~f:reset_instantiated
998-
| Type.Variable { constraints = Explicit annotations; _ } ->
999-
List.concat_map annotations ~f:(extract_access_data ~inside_meta)
1000-
|> List.map ~f:reset_instantiated
1001-
| Type.Variable { constraints = Bound bound; _ } ->
1002-
extract_access_data bound ~inside_meta
1003-
|> List.map ~f:reset_instantiated
1004-
| annotation when Type.is_meta annotation ->
1005-
Type.single_parameter annotation
1006-
|> extract_access_data ~inside_meta:true
1007-
| _ ->
1008-
begin
1009-
match Resolution.class_definition resolution annotation with
1010-
| Some class_definition ->
1011-
[{
1012-
instantiated = annotation;
1013-
class_attributes = inside_meta;
1014-
class_definition = Annotated.Class.create class_definition;
1015-
}]
1016-
| None ->
1017-
raise (TypeWithoutClass annotation)
1018-
end
988+
let was_variable, annotation =
989+
match annotation with
990+
| Type.Variable variable -> true, Type.upper_bound variable
991+
| _ -> false, annotation
992+
in
993+
let data =
994+
match annotation with
995+
| Type.Top | Type.Bottom | Type.Any ->
996+
[]
997+
| Type.Union annotations ->
998+
List.concat_map annotations ~f:(extract_access_data ~inside_meta)
999+
| annotation when Type.equal annotation Type.ellipsis ->
1000+
[]
1001+
| annotation when Type.is_meta annotation ->
1002+
Type.single_parameter annotation
1003+
|> extract_access_data ~inside_meta:true
1004+
| _ ->
1005+
begin
1006+
match Resolution.class_definition resolution annotation with
1007+
| Some class_definition ->
1008+
[{
1009+
instantiated = annotation;
1010+
class_attributes = inside_meta;
1011+
class_definition = Annotated.Class.create class_definition;
1012+
}]
1013+
| None ->
1014+
raise (TypeWithoutClass annotation)
1015+
end
1016+
in
1017+
if was_variable then List.map data ~f:reset_instantiated else data
10191018
in
10201019
let parent_annotation = Annotation.annotation resolved in
10211020
let attribute_step ~definition ~resolved =

‎analysis/typeOrder.ml

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -983,24 +983,19 @@ module OrderImplementation = struct
983983
left
984984

985985
(* We have to consider both the variables' constraint and its full value against the union. *)
986-
| Type.Variable { constraints = Type.Explicit constraints; _ }, Type.Union union ->
986+
| Type.Variable variable, Type.Union union ->
987987
List.exists ~f:(fun right -> less_or_equal order ~left ~right) union ||
988-
less_or_equal order ~left:(Type.union constraints) ~right
989-
| Type.Variable { constraints = Type.Bound bound; _ }, Type.Union union ->
990-
List.exists ~f:(fun right -> less_or_equal order ~left ~right) union ||
991-
less_or_equal order ~left:bound ~right
988+
less_or_equal order ~left:(Type.upper_bound variable) ~right
992989

993990
(* \exists i \in Union[...]. A <= B_i -> A <= Union[...] *)
994991
| left, Type.Union right ->
995992
List.exists ~f:(fun right -> less_or_equal order ~left ~right) right
996993

997994
(* We have to consider both the variables' constraint and its full value against the
998995
optional. *)
999-
| Type.Variable { constraints = Type.Explicit constraints; _ }, Type.Optional optional ->
996+
| Type.Variable variable, Type.Optional optional ->
1000997
less_or_equal order ~left ~right:optional ||
1001-
less_or_equal order ~left:(Type.union constraints) ~right
1002-
| Type.Variable { constraints = Type.Bound bound; _ }, Type.Optional optional ->
1003-
less_or_equal order ~left ~right:optional || less_or_equal order ~left:bound ~right
998+
less_or_equal order ~left:(Type.upper_bound variable) ~right
1004999

10051000
(* A <= B -> A <= Optional[B].*)
10061001
| Type.Optional left, Type.Optional right ->
@@ -1010,12 +1005,8 @@ module OrderImplementation = struct
10101005
| Type.Optional _, _ ->
10111006
false
10121007

1013-
| Type.Variable { constraints = Type.Explicit left; _ }, right ->
1014-
less_or_equal order ~left:(Type.union left) ~right
1015-
| Type.Variable { constraints = Type.Bound left; _ }, right ->
1016-
less_or_equal order ~left ~right
1017-
| Type.Variable { constraints = Type.Unconstrained; _ }, _ ->
1018-
false
1008+
| Type.Variable variable, right ->
1009+
less_or_equal order ~left:(Type.upper_bound variable) ~right
10191010

10201011
(* Tuple variables are covariant. *)
10211012
| Type.Tuple (Type.Bounded left), Type.Tuple (Type.Bounded right)
@@ -1357,17 +1348,9 @@ module OrderImplementation = struct
13571348
|> List.fold ~f:(join order) ~init:Type.Bottom
13581349
end
13591350

1360-
| (Type.Variable { constraints = Type.Unconstrained; _ } as variable), other
1361-
| other, (Type.Variable { constraints = Type.Unconstrained; _ } as variable) ->
1362-
Type.union [variable; other]
1363-
| Type.Variable { constraints = Type.Bound left; _ }, right ->
1364-
join order left right
1365-
| left, Type.Variable { constraints = Type.Bound right; _ } ->
1366-
join order left right
1367-
| Type.Variable { constraints = Type.Explicit left; _ }, right ->
1368-
join order (Type.union left) right
1369-
| left, Type.Variable { constraints = Type.Explicit right; _ } ->
1370-
join order left (Type.union right)
1351+
| other, Type.Variable variable
1352+
| Type.Variable variable, other ->
1353+
join order (Type.upper_bound variable) other
13711354

13721355
| Type.Parametric _, Type.Parametric _
13731356
| Type.Parametric _, Type.Primitive _

0 commit comments

Comments
 (0)