Skip to content

Commit 4fbe5bb

Browse files
committed
Fix option unboxing logic in the presence of untagged variants
Fixes #6222
1 parent 1c0bbf1 commit 4fbe5bb

File tree

5 files changed

+125
-14
lines changed

5 files changed

+125
-14
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#### :bug: Bug Fix
2020

2121
- Remove unnecessary require and import statements when using dynamic imports. https://github.com/rescript-lang/rescript-compiler/pull/6232
22+
- Fix option unboxing logic in the presence of untagged variants. https://github.com/rescript-lang/rescript-compiler/pull/6233
2223

2324
#### :nail_care: Polish
2425

jscomp/ml/ast_untagged_variants.ml

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,12 @@ let () =
9999
None
100100
)
101101

102+
let type_is_builtin_object (t:Types.type_expr) = match t.desc with
103+
| Tconstr (path, _, _) ->
104+
let name = Path.name path in
105+
name = "Js.Dict.t" || name = "Js_dict.t"
106+
| _ -> false
107+
102108
let get_block_type ~env (cstr: Types.constructor_declaration) : block_type option =
103109
match process_untagged cstr.cd_attributes, cstr.cd_args with
104110
| false, _ -> None
@@ -112,9 +118,7 @@ let get_block_type ~env (cstr: Types.constructor_declaration) : block_type optio
112118
Some ArrayType
113119
| true, Cstr_tuple [{desc = Tconstr (path, _, _)}] when Path. same path Predef.path_string ->
114120
Some StringType
115-
| true, Cstr_tuple [{desc = Tconstr (path, _, _)}] when
116-
let name = Path.name path in
117-
name = "Js.Dict.t" || name = "Js_dict.t" ->
121+
| true, Cstr_tuple [{desc = Tconstr _} as t] when type_is_builtin_object t ->
118122
Some ObjectType
119123
| true, Cstr_tuple [ty] ->
120124
let default = Some UnknownType in
@@ -242,6 +246,12 @@ let names_from_type_variant ?(isUntaggedDef=false) ~env (cstrs : Types.construct
242246
let check_well_formed ~env ~isUntaggedDef (cstrs: Types.constructor_declaration list) =
243247
ignore (names_from_type_variant ~env ~isUntaggedDef cstrs)
244248

249+
let has_undefined_literal attrs =
250+
process_tag_type attrs = Some Undefined
251+
252+
let block_is_object ~env attrs =
253+
get_block_type ~env attrs = Some ObjectType
254+
245255
module DynamicChecks = struct
246256

247257
type op = EqEqEq | NotEqEq | Or | And

jscomp/ml/typeopt.ml

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,34 +44,57 @@ let scrape env ty =
4444
records the type at the definition type so for ['a option]
4545
it will always be [Tvar]
4646
*)
47-
let cannot_inhabit_none_like_value (typ : Types.type_expr) (env : Env.t) =
47+
let rec cannot_inhabit_none_like_value (typ : Types.type_expr) (env : Env.t) =
4848
match scrape env typ with
4949
| Tconstr(p, _,_) ->
5050
(* all built in types could not inhabit none-like values:
5151
int, char, float, bool, unit, exn, array, list, nativeint,
5252
int32, int64, lazy_t, bytes
5353
*)
5454
(match Predef.type_is_builtin_path_but_option p with
55-
| For_sure_yes -> true
55+
| For_sure_yes -> true
5656
| For_sure_no -> false
57-
| NA ->
58-
59-
begin match (Env.find_type p env).type_kind with
57+
| NA ->
58+
let untagged = ref false in
59+
begin match
60+
let decl = Env.find_type p env in
61+
let () =
62+
if Ast_untagged_variants.has_untagged decl.type_attributes
63+
then untagged := true in
64+
decl.type_kind with
6065
| exception _ ->
6166
false
62-
| Types.Type_abstract | Types.Type_open -> false
63-
| Types.Type_record _ -> true
64-
| (Types.Type_variant
67+
| Type_abstract | Type_open -> false
68+
| Type_record _ -> true
69+
| Type_variant
6570
([{cd_id = {name="None"}; cd_args = Cstr_tuple [] };
6671
{cd_id = {name = "Some"}; cd_args = Cstr_tuple [_]}]
6772
|
6873
[{cd_id = {name="Some"}; cd_args = Cstr_tuple [_] };
6974
{cd_id = {name = "None"}; cd_args = Cstr_tuple []}]
7075
| [{cd_id= {name = "()"}; cd_args = Cstr_tuple []}]
71-
))
72-
(* | Types.Type_variant *)
76+
)
7377
-> false (* conservative *)
74-
| _ -> true
78+
| Type_variant cdecls ->
79+
let type_can_contain_undefined t =
80+
not (Ast_untagged_variants.type_is_builtin_object t) &&
81+
not (cannot_inhabit_none_like_value t env) in
82+
let can_contain_undefined =
83+
Ext_list.exists cdecls (fun cd ->
84+
if Ast_untagged_variants.has_undefined_literal cd.cd_attributes
85+
then true
86+
else if !untagged then
87+
match cd.cd_args with
88+
| Cstr_tuple [t] ->
89+
type_can_contain_undefined t
90+
| Cstr_tuple [] -> false
91+
| Cstr_tuple (_::_::_) -> false (* Not actually possible for untagged *)
92+
| Cstr_record [{ld_type=t}] ->
93+
type_can_contain_undefined t
94+
| Cstr_record ([] | _::_::_) -> false
95+
else
96+
false) in
97+
not can_contain_undefined
7598
end)
7699
| Ttuple _
77100
| Tvariant _

jscomp/test/UntaggedVariants.js

Lines changed: 45 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

jscomp/test/UntaggedVariants.res

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,3 +261,35 @@ module ArrayAndObject = {
261261
| Array(a) => a[0]
262262
}
263263
}
264+
265+
module OptionUnboxingHeuristic = {
266+
type hasNull = | @as(null) Null | B(int)
267+
let testHasNull = (x: hasNull) => Some(x)
268+
269+
type hasUndefined = | @as(undefined) Undefined | B(int)
270+
let testHasUndefined = (x: hasUndefined) => Some(x)
271+
272+
@unboxed
273+
type untaggedWithOptionPayload = A | B(option<string>)
274+
let untaggedWithOptionPayload = (x: untaggedWithOptionPayload) => Some(x)
275+
276+
@unboxed
277+
type untaggedWithIntPayload = A | B(int)
278+
let untaggedWithIntPayload = (x: untaggedWithIntPayload) => Some(x)
279+
280+
@unboxed
281+
type untaggedInlineNoOption = A | B({x: int})
282+
let untaggedInlineNoOptions = (x: untaggedInlineNoOption) => Some(x)
283+
284+
@unboxed
285+
type untaggedInlineUnaryWihtExplicitOption = A | B({x: option<int>})
286+
let untaggedInlineUnaryWihtExplicitOption = (x: untaggedInlineUnaryWihtExplicitOption) => Some(x)
287+
288+
@unboxed
289+
type untaggedInlineUnaryWihtImplicitOption = A | B({x?: int})
290+
let untaggedInlineUnaryWihtImplicitOption = (x: untaggedInlineUnaryWihtImplicitOption) => Some(x)
291+
292+
@unboxed
293+
type untaggedInlineMultinaryOption = A | B({x: option<int>, y?: string})
294+
let untaggedInlineMultinaryOption = (x: untaggedInlineMultinaryOption) => Some(x)
295+
}

0 commit comments

Comments
 (0)