Skip to content

Don't emit f(undefined) when passing unit (). #6459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
- Add [`Deno`](https://deno.land/api?s=Deno) to reserved names, so that modules named `Deno` don't clash with the globally exposed `Deno` object. https://github.com/rescript-lang/rescript-compiler/pull/6428
- Disable ESLint/TSLint on gentype outputs properly. https://github.com/rescript-lang/rescript-compiler/pull/6442
- Improve `rescript` CLI to use `stdout`/`stderr` appropriately for help command's message. https://github.com/rescript-lang/rescript-compiler/pull/6439
- Generate `f()` instead of `f(undefined)` for `f()` https://github.com/rescript-lang/rescript-compiler/pull/6459

# 11.0.0-rc.4

Expand Down
2 changes: 1 addition & 1 deletion jscomp/core/j.ml
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ and expression_desc =
*)
| Number of number
| Object of property_map
| Undefined
| Undefined of {isUnit: bool}
| Null
| Await of expression

Expand Down
4 changes: 2 additions & 2 deletions jscomp/core/js_analyzer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ let free_variables_of_expression st =

let rec no_side_effect_expression_desc (x : J.expression_desc) =
match x with
| Undefined | Null | Bool _ | Var _ -> true
| Undefined _ | Null | Bool _ | Var _ -> true
| Fun _ -> true
| Number _ -> true (* Can be refined later *)
| Static_index (obj, (_name : string), (_pos : int32 option)) ->
Expand Down Expand Up @@ -153,7 +153,7 @@ let rec eq_expression ({ expression_desc = x0 } : J.expression)
({ expression_desc = y0 } : J.expression) =
match x0 with
| Null -> y0 = Null
| Undefined -> y0 = Undefined
| Undefined x -> y0 = Undefined x
| Number (Int { i }) -> (
match y0 with Number (Int { i = j }) -> i = j | _ -> false)
| Number (Float _) -> false
Expand Down
2 changes: 1 addition & 1 deletion jscomp/core/js_cmj_format.ml
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ let get_result midVal =
match midVal.persistent_closed_lambda with
| Some
(Lconst
(Const_js_null | Const_js_undefined | Const_js_true | Const_js_false))
(Const_js_null | Const_js_undefined _ | Const_js_true | Const_js_false))
| None ->
midVal
| Some _ ->
Expand Down
25 changes: 16 additions & 9 deletions jscomp/core/js_dump.ml
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ let exp_need_paren (e : J.expression) =
| Raw_js_code { code_info = Stmt _ }
| Length _ | Call _ | Caml_block_tag _ | Seq _ | Static_index _ | Cond _
| Bin _ | Is_null_or_undefined _ | String_index _ | Array_index _
| String_append _ | Var _ | Undefined | Null | Str _ | Array _
| String_append _ | Var _ | Undefined _ | Null | Str _ | Array _
| Optional_block _ | Caml_block _ | FlatCall _ | Typeof _ | Number _
| Js_not _ | Bool _ | New _ ->
false
Expand Down Expand Up @@ -519,7 +519,7 @@ and expression_desc cxt ~(level : int) f x : cxt =
| Null ->
P.string f L.null;
cxt
| Undefined ->
| Undefined _ ->
P.string f L.undefined;
cxt
| Var v -> vident cxt f v
Expand Down Expand Up @@ -569,7 +569,14 @@ and expression_desc cxt ~(level : int) f x : cxt =
pp_function ~is_method ~return_unit ~async cxt f
~fn_state:(No_name { single_arg = true })
params body env
| _ -> arguments cxt f el)
| _ ->
let el = match el with
| [e] when e.expression_desc = Undefined {isUnit = true} ->
(* omit passing undefined when the call is f() *)
[]
| _ ->
el in
arguments cxt f el)
| _, _ ->
let len = List.length el in
if 1 <= len && len <= 8 then (
Expand Down Expand Up @@ -736,7 +743,7 @@ and expression_desc cxt ~(level : int) f x : cxt =
let fields =
Ext_list.array_list_filter_map fields el (fun f x ->
match x.expression_desc with
| Undefined -> None
| Undefined _ -> None
| _ -> Some (Js_op.Lit f, x))
in
expression_desc cxt ~level f (Object fields))
Expand Down Expand Up @@ -774,7 +781,7 @@ and expression_desc cxt ~(level : int) f x : cxt =
| _ ->
Ext_list.filter_map tails (fun (f, x) ->
match x.expression_desc with
| Undefined when is_optional f -> None
| Undefined _ when is_optional f -> None
| _ -> Some (f, x))
in
if untagged then
Expand Down Expand Up @@ -1176,7 +1183,7 @@ and statement_desc top cxt f (s : J.statement_desc) : cxt =
in
semi f;
cxt
| Undefined ->
| Undefined _ ->
return_sp f;
semi f;
cxt
Expand Down Expand Up @@ -1270,16 +1277,16 @@ and function_body (cxt : cxt) f ~return_unit (b : J.block) : unit =
| If
( bool,
then_,
[ { statement_desc = Return { expression_desc = Undefined } } ] ) ->
[ { statement_desc = Return { expression_desc = Undefined _ } } ] ) ->
ignore
(statement false cxt f
{ s with statement_desc = If (bool, then_, []) }
: cxt)
| Return { expression_desc = Undefined } -> ()
| Return { expression_desc = Undefined _ } -> ()
| Return exp when return_unit ->
ignore (statement false cxt f (S.exp exp) : cxt)
| _ -> ignore (statement false cxt f s : cxt))
| [ s; { statement_desc = Return { expression_desc = Undefined } } ] ->
| [ s; { statement_desc = Return { expression_desc = Undefined _ } } ] ->
ignore (statement false cxt f s : cxt)
| s :: r ->
let cxt = statement false cxt f s in
Expand Down
64 changes: 32 additions & 32 deletions jscomp/core/js_exp_make.ml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ let var ?comment id : t = { expression_desc = Var (Id id); comment }
Invariant: it should not call an external module .. *)

let js_global ?comment (v : string) = var ?comment (Ext_ident.create_js v)
let undefined : t = { expression_desc = Undefined; comment = None }
let undefined : t = { expression_desc = Undefined {isUnit = false}; comment = None }
let nil : t = { expression_desc = Null; comment = None }

let call ?comment ~info e0 args : t =
Expand Down Expand Up @@ -183,7 +183,7 @@ let is_array (e0 : t) : t =
let new_ ?comment e0 args : t =
{ expression_desc = New (e0, Some args); comment }

let unit : t = { expression_desc = Undefined; comment = None }
let unit : t = { expression_desc = Undefined {isUnit = true}; comment = None }

(* let math ?comment v args : t =
{comment ; expression_desc = Math(v,args)} *)
Expand Down Expand Up @@ -256,17 +256,17 @@ let dummy_obj ?comment (info : Lam_tag_info.t) : t =
*)
let rec seq ?comment (e0 : t) (e1 : t) : t =
match (e0.expression_desc, e1.expression_desc) with
| ( ( Seq (a, { expression_desc = Number _ | Undefined })
| Seq ({ expression_desc = Number _ | Undefined }, a) ),
| ( ( Seq (a, { expression_desc = Number _ | Undefined _ })
| Seq ({ expression_desc = Number _ | Undefined _ }, a) ),
_ ) ->
seq ?comment a e1
| _, Seq ({ expression_desc = Number _ | Undefined }, a) ->
| _, Seq ({ expression_desc = Number _ | Undefined _ }, a) ->
(* Return value could not be changed*)
seq ?comment e0 a
| _, Seq (a, ({ expression_desc = Number _ | Undefined } as v)) ->
| _, Seq (a, ({ expression_desc = Number _ | Undefined _ } as v)) ->
(* Return value could not be changed*)
seq ?comment (seq e0 a) v
| (Number _ | Var _ | Undefined), _ -> e1
| (Number _ | Var _ | Undefined _), _ -> e1
| _ -> { expression_desc = Seq (e0, e1); comment }

let fuse_to_seq x xs = if xs = [] then x else Ext_list.fold_left xs x seq
Expand Down Expand Up @@ -567,22 +567,22 @@ let str_equal (txt0:string) (delim0:External_arg_spec.delim) txt1 delim1 =

let rec triple_equal ?comment (e0 : t) (e1 : t) : t =
match (e0.expression_desc, e1.expression_desc) with
| ( (Null | Undefined),
| ( (Null | Undefined _),
(Bool _ | Number _ | Typeof _ | Fun _ | Array _ | Caml_block _) )
when no_side_effect e1 ->
false_
| ( (Bool _ | Number _ | Typeof _ | Fun _ | Array _ | Caml_block _),
(Null | Undefined) )
(Null | Undefined _) )
when no_side_effect e0 ->
false_
| Number (Int { i = i0; _ }), Number (Int { i = i1; _ }) -> bool (i0 = i1)
| Optional_block (a, _), Optional_block (b, _) -> triple_equal ?comment a b
| Undefined, Optional_block _
| Optional_block _, Undefined
| Null, Undefined
| Undefined, Null ->
| Undefined _, Optional_block _
| Optional_block _, Undefined _
| Null, Undefined _
| Undefined _, Null ->
false_
| Null, Null | Undefined, Undefined -> true_
| Null, Null | Undefined _, Undefined _ -> true_
| _ -> { expression_desc = Bin (EqEqEq, e0, e1); comment }

let bin ?comment (op : J.binop) (e0 : t) (e1 : t) : t =
Expand Down Expand Up @@ -654,7 +654,7 @@ let and_ ?comment (e1 : t) (e2 : t) : t =
| Var i, Bin (And, l, ({ expression_desc = Var j; _ } as r))
when Js_op_util.same_vident i j ->
{ e2 with expression_desc = Bin (And, r, l) }
| ( Bin (NotEqEq, { expression_desc = Var i }, { expression_desc = Undefined }),
| ( Bin (NotEqEq, { expression_desc = Var i }, { expression_desc = Undefined _ }),
Bin
( EqEqEq,
{ expression_desc = Var j },
Expand Down Expand Up @@ -702,7 +702,7 @@ let not (e : t) : t =

let not_empty_branch (x : t) =
match x.expression_desc with
| Number (Int { i = 0l }) | Undefined -> false
| Number (Int { i = 0l }) | Undefined _ -> false
| _ -> true

let rec econd ?comment (pred : t) (ifso : t) (ifnot : t) : t =
Expand Down Expand Up @@ -735,8 +735,8 @@ let rec econd ?comment (pred : t) (ifso : t) (ifnot : t) : t =
econd (or_ pred (not pred1)) ifso ifso1
| Js_not e, _, _ when not_empty_branch ifnot -> econd ?comment e ifnot ifso
| ( _,
Seq (a, { expression_desc = Undefined }),
Seq (b, { expression_desc = Undefined }) ) ->
Seq (a, { expression_desc = Undefined _ }),
Seq (b, { expression_desc = Undefined _ }) ) ->
seq (econd ?comment pred a b) undefined
| _ ->
if Js_analyzer.eq_expression ifso ifnot then
Expand All @@ -746,7 +746,7 @@ let rec econd ?comment (pred : t) (ifso : t) (ifnot : t) : t =
let rec float_equal ?comment (e0 : t) (e1 : t) : t =
match (e0.expression_desc, e1.expression_desc) with
| Number (Int { i = i0; _ }), Number (Int { i = i1 }) -> bool (i0 = i1)
| Undefined, Undefined -> true_
| Undefined _, Undefined _ -> true_
(* | (Bin(Bor,
{expression_desc = Number(Int {i = 0l; _})},
({expression_desc = Caml_block_tag _; _} as a ))
Expand Down Expand Up @@ -983,11 +983,11 @@ let rec int_comp (cmp : Lam_compat.comparison) ?comment (e0 : t) (e1 : t) =
args,
call_info );
}
| Ceq, Optional_block _, Undefined | Ceq, Undefined, Optional_block _ ->
| Ceq, Optional_block _, Undefined _ | Ceq, Undefined _, Optional_block _ ->
false_
| Ceq, _, _ -> int_equal e0 e1
| Cneq, Optional_block _, Undefined
| Cneq, Undefined, Optional_block _
| Cneq, Optional_block _, Undefined _
| Cneq, Undefined _, Optional_block _
| Cneq, Caml_block _, Number _
| Cneq, Number _, Caml_block _ ->
true_
Expand Down Expand Up @@ -1281,36 +1281,36 @@ let is_null ?comment (x : t) = triple_equal ?comment x nil
let is_undef ?comment x = triple_equal ?comment x undefined

let for_sure_js_null_undefined (x : t) =
match x.expression_desc with Null | Undefined -> true | _ -> false
match x.expression_desc with Null | Undefined _ -> true | _ -> false

let is_null_undefined ?comment (x : t) : t =
match x.expression_desc with
| Null | Undefined -> true_
| Null | Undefined _ -> true_
| Number _ | Array _ | Caml_block _ -> false_
| _ -> { comment; expression_desc = Is_null_or_undefined x }

let eq_null_undefined_boolean ?comment (a : t) (b : t) =
match (a.expression_desc, b.expression_desc) with
| ( (Null | Undefined),
| ( (Null | Undefined _),
(Bool _ | Number _ | Typeof _ | Fun _ | Array _ | Caml_block _) ) ->
false_
| ( (Bool _ | Number _ | Typeof _ | Fun _ | Array _ | Caml_block _),
(Null | Undefined) ) ->
(Null | Undefined _) ) ->
false_
| Null, Undefined | Undefined, Null -> false_
| Null, Null | Undefined, Undefined -> true_
| Null, Undefined _ | Undefined _, Null -> false_
| Null, Null | Undefined _, Undefined _ -> true_
| _ -> { expression_desc = Bin (EqEqEq, a, b); comment }

let neq_null_undefined_boolean ?comment (a : t) (b : t) =
match (a.expression_desc, b.expression_desc) with
| ( (Null | Undefined),
| ( (Null | Undefined _),
(Bool _ | Number _ | Typeof _ | Fun _ | Array _ | Caml_block _) ) ->
true_
| ( (Bool _ | Number _ | Typeof _ | Fun _ | Array _ | Caml_block _),
(Null | Undefined) ) ->
(Null | Undefined _) ) ->
true_
| Null, Null | Undefined, Undefined -> false_
| Null, Undefined | Undefined, Null -> true_
| Null, Null | Undefined _, Undefined _ -> false_
| Null, Undefined _ | Undefined _, Null -> true_
| _ -> { expression_desc = Bin (NotEqEq, a, b); comment }

(** TODO: in the future add a flag
Expand Down
2 changes: 1 addition & 1 deletion jscomp/core/js_fold.ml
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ class fold =
| Object _x0 ->
let _self = _self#property_map _x0 in
_self
| Undefined -> _self
| Undefined _ -> _self
| Null -> _self
| Await _x0 ->
let _self = _self#expression _x0 in
Expand Down
4 changes: 3 additions & 1 deletion jscomp/core/js_of_lam_option.ml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ type option_unwrap_time = Static_unwrapped | Runtime_maybe_unwrapped
*)
let none : J.expression = E.undefined

let is_none_static (arg : J.expression_desc) = arg = Undefined
let is_none_static (arg : J.expression_desc) = match arg with
| Undefined _ -> true
| _ -> false

let is_not_none (e : J.expression) : J.expression =
let desc = e.expression_desc in
Expand Down
4 changes: 2 additions & 2 deletions jscomp/core/js_pass_flatten_and_mark_dead.ml
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ let subst_map (substitution : J.expression Hash_ident.t) =
let _, e, bindings =
Ext_list.fold_left ls (0, [], []) (fun (i, e, acc) x ->
match x.expression_desc with
| Var _ | Number _ | Str _ | J.Bool _ | Undefined ->
| Var _ | Number _ | Str _ | J.Bool _ | Undefined _ ->
(* TODO: check the optimization *)
(i + 1, x :: e, acc)
| _ ->
Expand Down Expand Up @@ -257,7 +257,7 @@ let subst_map (substitution : J.expression Hash_ident.t) =
match Ext_list.nth_opt ls (Int32.to_int i) with
| Some
({
expression_desc = J.Var _ | Number _ | Str _ | Undefined;
expression_desc = J.Var _ | Number _ | Str _ | Undefined _;
} as x) ->
x
| None | Some _ -> super.expression self x)
Expand Down
2 changes: 1 addition & 1 deletion jscomp/core/js_record_fold.ml
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ let expression_desc : 'a. ('a, expression_desc) fn =
| Object _x0 ->
let st = property_map _self st _x0 in
st
| Undefined -> st
| Undefined _ -> st
| Null -> st
| Await _x0 ->
let st = _self.expression _self st _x0 in
Expand Down
2 changes: 1 addition & 1 deletion jscomp/core/js_record_iter.ml
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ let expression_desc : expression_desc fn =
| Caml_block_tag (_x0, _tag) -> _self.expression _self _x0
| Number _ -> ()
| Object _x0 -> property_map _self _x0
| Undefined -> ()
| Undefined _ -> ()
| Null -> ()
| Await _x0 -> _self.expression _self _x0

Expand Down
2 changes: 1 addition & 1 deletion jscomp/core/js_record_map.ml
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ let expression_desc : expression_desc fn =
| Object _x0 ->
let _x0 = property_map _self _x0 in
Object _x0
| Undefined as v -> v
| Undefined _ as v -> v
| Null as v -> v
| Await _x0 ->
let _x0 = _self.expression _self _x0 in
Expand Down
12 changes: 6 additions & 6 deletions jscomp/core/js_stmt_make.ml
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ let rec block ?comment (b : J.block) : t =
(* It's a statement, we can discard some values *)
let rec exp ?comment (e : E.t) : t =
match e.expression_desc with
| Seq ({ expression_desc = Number _ | Undefined }, b)
| Seq (b, { expression_desc = Number _ | Undefined }) ->
| Seq ({ expression_desc = Number _ | Undefined _ }, b)
| Seq (b, { expression_desc = Number _ | Undefined _ }) ->
exp ?comment b
| Number _ | Undefined -> block []
| Number _ | Undefined _ -> block []
(* TODO: we can do more *)
(* | _ when is_pure e -> block [] *)
| _ -> { statement_desc = Exp e; comment }
Expand All @@ -63,10 +63,10 @@ let declare_variable ?comment ?ident_info ~kind (ident : Ident.t) : t =
}

let define_variable ?comment ?ident_info ~kind (v : Ident.t)
(exp : J.expression) : t =
if exp.expression_desc = Undefined then
(exp : J.expression) : t = match exp.expression_desc with
| Undefined _ ->
declare_variable ?comment ?ident_info ~kind v
else
| _ ->
let property : J.property = kind in
let ident_info : J.ident_info =
match ident_info with None -> { used_stats = NA } | Some x -> x
Expand Down
Loading