Skip to content
Open
Show file tree
Hide file tree
Changes from 8 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 @@ -16,6 +16,7 @@

- Make `Jsx.component` abstract. https://github.com/rescript-lang/rescript/pull/8390
- Drop Node.js version 20.x support, as it is reaching EOL. https://github.com/rescript-lang/rescript/pull/8401
- Remove `assert` as a reserved keyword. Since v12 `assert` is parsed as regular function. If you ran the formatter on your codebase using v12, this change is not a breaking change. https://github.com/rescript-lang/rescript/pull/8399

#### :eyeglasses: Spec Compliance

Expand Down
2 changes: 0 additions & 2 deletions analysis/src/DumpAst.ml
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,6 @@ and printExprItem expr ~pos ~indentation =
^ "\n" ^ addIndentation indentation ^ ")"
| Pexp_extension (({txt} as loc), _) ->
"Pexp_extension(%" ^ (loc |> printLocDenominatorLoc ~pos) ^ txt ^ ")"
| Pexp_assert expr ->
"Pexp_assert(" ^ printExprItem expr ~pos ~indentation ^ ")"
| Pexp_field (exp, loc) ->
"Pexp_field("
^ (loc |> printLocDenominatorLoc ~pos)
Expand Down
1 change: 0 additions & 1 deletion analysis/src/Utils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ let identifyPexp pexp =
| Pexp_send _ -> "Pexp_send"
| Pexp_letmodule _ -> "Pexp_letmodule"
| Pexp_letexception _ -> "Pexp_letexception"
| Pexp_assert _ -> "Pexp_assert"
| Pexp_newtype _ -> "Pexp_newtype"
| Pexp_pack _ -> "Pexp_pack"
| Pexp_extension _ -> "Pexp_extension"
Expand Down
1 change: 0 additions & 1 deletion compiler/frontend/bs_ast_mapper.ml
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,6 @@ module E = struct
letexception ~loc ~attrs
(sub.extension_constructor sub cd)
(sub.expr sub e)
| Pexp_assert e -> assert_ ~loc ~attrs (sub.expr sub e)
| Pexp_newtype (s, e) ->
newtype ~loc ~attrs (map_loc sub s) (sub.expr sub e)
| Pexp_pack me -> pack ~loc ~attrs (sub.module_expr sub me)
Expand Down
1 change: 0 additions & 1 deletion compiler/ml/ast_helper.ml
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ module Exp = struct
let send ?loc ?attrs a b = mk ?loc ?attrs (Pexp_send (a, b))
let letmodule ?loc ?attrs a b c = mk ?loc ?attrs (Pexp_letmodule (a, b, c))
let letexception ?loc ?attrs a b = mk ?loc ?attrs (Pexp_letexception (a, b))
let assert_ ?loc ?attrs a = mk ?loc ?attrs (Pexp_assert a)
let newtype ?loc ?attrs a b = mk ?loc ?attrs (Pexp_newtype (a, b))
let pack ?loc ?attrs a = mk ?loc ?attrs (Pexp_pack a)
let open_ ?loc ?attrs a b c = mk ?loc ?attrs (Pexp_open (a, b, c))
Expand Down
1 change: 0 additions & 1 deletion compiler/ml/ast_helper.mli
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,6 @@ module Exp : sig
extension_constructor ->
expression ->
expression
val assert_ : ?loc:loc -> ?attrs:attrs -> expression -> expression
val newtype : ?loc:loc -> ?attrs:attrs -> str -> expression -> expression
val pack : ?loc:loc -> ?attrs:attrs -> module_expr -> expression
val open_ :
Expand Down
1 change: 0 additions & 1 deletion compiler/ml/ast_iterator.ml
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,6 @@ module E = struct
| Pexp_letexception (cd, e) ->
sub.extension_constructor sub cd;
sub.expr sub e
| Pexp_assert e -> sub.expr sub e
| Pexp_newtype (_s, e) -> sub.expr sub e
| Pexp_pack me -> sub.module_expr sub me
| Pexp_open (_ovf, lid, e) ->
Expand Down
1 change: 0 additions & 1 deletion compiler/ml/ast_mapper.ml
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,6 @@ module E = struct
letexception ~loc ~attrs
(sub.extension_constructor sub cd)
(sub.expr sub e)
| Pexp_assert e -> assert_ ~loc ~attrs (sub.expr sub e)
| Pexp_newtype (s, e) ->
newtype ~loc ~attrs (map_loc sub s) (sub.expr sub e)
| Pexp_pack me -> pack ~loc ~attrs (sub.module_expr sub me)
Expand Down
6 changes: 5 additions & 1 deletion compiler/ml/ast_mapper_from0.ml
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,11 @@ module E = struct
letexception ~loc ~attrs
(sub.extension_constructor sub cd)
(sub.expr sub e)
| Pexp_assert e -> assert_ ~loc ~attrs (sub.expr sub e)
| Pexp_assert e ->
apply ~loc ~attrs
(ident ~loc
{txt = Longident.Ldot (Longident.Lident "Pervasives", "assert"); loc})
[(Asttypes.Nolabel, sub.expr sub e)]
| Pexp_lazy _ -> failwith "Pexp_lazy is no longer present in ReScript"
| Pexp_poly _ -> failwith "Pexp_poly is no longer present in ReScript"
| Pexp_object () -> assert false
Expand Down
1 change: 0 additions & 1 deletion compiler/ml/ast_mapper_to0.ml
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,6 @@ module E = struct
letexception ~loc ~attrs
(sub.extension_constructor sub cd)
(sub.expr sub e)
| Pexp_assert e -> assert_ ~loc ~attrs (sub.expr sub e)
| Pexp_newtype (s, e) ->
newtype ~loc ~attrs (map_loc sub s) (sub.expr sub e)
| Pexp_pack me -> pack ~loc ~attrs (sub.module_expr sub me)
Expand Down
1 change: 0 additions & 1 deletion compiler/ml/depend.ml
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,6 @@ let rec add_expr bv exp =
let b = add_module_binding bv m in
add_expr (StringMap.add id.txt b bv) e
| Pexp_letexception (_, e) -> add_expr bv e
| Pexp_assert e -> add_expr bv e
| Pexp_newtype (_, e) -> add_expr bv e
| Pexp_pack m -> add_module bv m
| Pexp_open (_ovf, m, e) ->
Expand Down
4 changes: 0 additions & 4 deletions compiler/ml/parsetree.ml
Original file line number Diff line number Diff line change
Expand Up @@ -303,10 +303,6 @@ and expression_desc =
(* let module M = ME in E *)
| Pexp_letexception of extension_constructor * expression
(* let exception C in E *)
| Pexp_assert of expression
(* assert E
Note: "assert false" is treated in a special way by the
type-checker. *)
| Pexp_newtype of string loc * expression (* fun (type t) -> E *)
| Pexp_pack of module_expr
(* (module ME)
Expand Down
1 change: 0 additions & 1 deletion compiler/ml/pprintast.ml
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,6 @@ and expression ctxt f x =
pp f "@[<hov2>let@ exception@ %a@ in@ %a@]"
(extension_constructor ctxt)
cd (expression ctxt) e
| Pexp_assert e -> pp f "@[<hov2>assert@ %a@]" (simple_expr ctxt) e
| Pexp_open (ovf, lid, e) ->
pp f "@[<2>let open%s %a in@;%a@]" (override ovf) longident_loc lid
(expression ctxt) e
Expand Down
3 changes: 0 additions & 3 deletions compiler/ml/printast.ml
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,6 @@ and expression i ppf x =
line i ppf "Pexp_letexception\n";
extension_constructor i ppf cd;
expression i ppf e
| Pexp_assert e ->
line i ppf "Pexp_assert\n";
expression i ppf e
| Pexp_newtype (s, e) ->
line i ppf "Pexp_newtype \"%s\"\n" s.txt;
expression i ppf e
Expand Down
132 changes: 92 additions & 40 deletions compiler/ml/translcore.ml
Original file line number Diff line number Diff line change
Expand Up @@ -453,52 +453,87 @@ let warn_polymorphic_comparison loc prim args =
Location.prerr_warning loc Warnings.Bs_polymorphic_comparison
| _ -> ()

let assert_failed_at loc =
let fname, line, char = Location.get_pos_info loc.Location.loc_start in
let fname = Filename.basename fname in
Lprim
( Praise Raise_regular,
[
Lprim
( Pmakeblock Blk_extension,
[
transl_normal_path Predef.path_assert_failure;
Lconst
(Const_block
( Blk_tuple,
[
Const_base (Const_string (fname, None));
Const_base (Const_int line);
Const_base (Const_int char);
] ));
],
loc );
],
loc )

(* Eta-expand a primitive *)

let transl_primitive loc p env ty =
(* Printf.eprintf "----transl_primitive %s----\n" p.prim_name; *)
let prim =
try specialize_primitive p env ty (* ~has_constant_constructor:false *)
with Not_found -> Pccall p
in
warn_polymorphic_comparison loc prim [];
match prim with
| Ploc kind -> (
let lam = lam_of_loc kind loc in
match p.prim_arity with
| 0 -> lam
| 1 ->
(* TODO: we should issue a warning ? *)
let param = Ident.create "prim" in
Lfunction
{
params = [param];
attr = default_function_attribute;
loc;
body = Lprim (Pmakeblock Blk_tuple, [lam; Lvar param], loc);
}
| _ -> assert false)
| _ ->
let rec make_params n total =
if n <= 0 then []
else
Ident.create ("prim" ^ string_of_int (total - n))
:: make_params (n - 1) total
if p.Primitive.prim_name = "%assert" then
let param = Ident.create "assert_cond" in
Lfunction
{
params = [param];
attr = default_function_attribute;
loc;
body =
(if !Clflags.noassert then lambda_unit
else Lifthenelse (Lvar param, lambda_unit, assert_failed_at loc));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve -noassert behavior for first-class assert

When %assert is translated as a normal one-argument function here, calls through an alias still evaluate the argument even under -noassert (e.g. let f = assert; f(sideEffect())), because function arguments are evaluated before the call. That diverges from assertion-disabled semantics used elsewhere in this commit (direct assert(...) is lowered to () without evaluating the condition), so enabling -noassert no longer reliably removes assertion-condition side effects once assert is passed around as a value.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To preserve -noassert behavior %assert should be a non-first-class again.

To make first-class assert correct, we would need a much deeper special representation: track aliases of %assert, recognize applications through those aliases, possibly through records/modules/higher-order arguments, and lower the entire application before evaluating the condition.

We can add some limitation, assert can only be used as a direct call like assert(condition). It cannot be aliased or passed as a function.

What you think @cknitt @fhammerschmidt ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aspeddro Sounds like a good compromise - can you give it a try?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See 381abce

else
let prim =
try specialize_primitive p env ty (* ~has_constant_constructor:false *)
with Not_found -> Pccall p
in
let prim_arity = p.prim_arity in
if p.prim_from_constructor || prim_arity = 0 then Lprim (prim, [], loc)
else
let params =
if prim_arity = 1 then [Ident.create "prim"]
else make_params prim_arity prim_arity
warn_polymorphic_comparison loc prim [];
match prim with
| Ploc kind -> (
let lam = lam_of_loc kind loc in
match p.prim_arity with
| 0 -> lam
| 1 ->
(* TODO: we should issue a warning ? *)
let param = Ident.create "prim" in
Lfunction
{
params = [param];
attr = default_function_attribute;
loc;
body = Lprim (Pmakeblock Blk_tuple, [lam; Lvar param], loc);
}
| _ -> assert false)
| _ ->
let rec make_params n total =
if n <= 0 then []
else
Ident.create ("prim" ^ string_of_int (total - n))
:: make_params (n - 1) total
in
Lfunction
{
params;
attr = default_function_attribute;
loc;
body = Lprim (prim, List.map (fun id -> Lvar id) params, loc);
}
let prim_arity = p.prim_arity in
if p.prim_from_constructor || prim_arity = 0 then Lprim (prim, [], loc)
else
let params =
if prim_arity = 1 then [Ident.create "prim"]
else make_params prim_arity prim_arity
in
Lfunction
{
params;
attr = default_function_attribute;
loc;
body = Lprim (prim, List.map (fun id -> Lvar id) params, loc);
}

let transl_primitive_application loc prim env ty args =
let prim_name = prim.prim_name in
Expand Down Expand Up @@ -718,6 +753,23 @@ and transl_exp0 (e : Typedtree.expression) : Lambda.lambda =
[lambda],
loc )
| None -> lambda)
| Texp_apply
{
funct =
{
exp_desc =
Texp_ident (_, _, {val_kind = Val_prim {prim_name = "%assert"}});
};
args = [(_, Some cond)];
} -> (
(* assert(cond) — same semantics as the old assert keyword *)
match cond.exp_desc with
| Texp_construct (_, {cstr_name = "false"}, _) ->
if !Clflags.no_assert_false then Lambda.lambda_assert_false
else assert_failed e
| _ ->
if !Clflags.noassert then lambda_unit
else Lifthenelse (transl_exp cond, lambda_unit, assert_failed e))
| Texp_apply
{
funct =
Expand Down
42 changes: 23 additions & 19 deletions compiler/ml/typecore.ml
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ let iter_expression f e =
List.iter (fun {x = e} -> expr e) iel
| Pexp_open (_, _, e)
| Pexp_newtype (_, e)
| Pexp_assert e
| Pexp_send (e, _)
| Pexp_constraint (e, _)
| Pexp_coerce (e, _, _)
Expand Down Expand Up @@ -2503,6 +2502,29 @@ and type_expect_ ?deprecated_context ~context ?in_function ?(recarg = Rejected)
type_function ?in_function ~arity ~async loc sexp.pexp_attributes env
ty_expected l
[Ast_helper.Exp.case spat sbody]
| Pexp_apply {funct = {pexp_desc = Pexp_ident lid}; args = [(Nolabel, scond)]}
when match Env.lookup_value lid.txt env with
| _, {val_kind = Val_prim {Primitive.prim_name = "%assert"}} -> true
| _ -> false
| exception Not_found -> false ->
Comment on lines +2513 to +2516
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle %assert when used via aliases or higher-order values

This special-case only rewrites direct assert(cond) calls, so %assert can still escape as a first-class value (e.g. let f = assert; f(true)). In that path the backend no longer sees Texp_assert, and %assert falls through primitive lowering paths that are not implemented for it, which can surface as an internal primitive-conversion failure instead of normal assert semantics. Please either reject non-direct %assert usage in typing or add full primitive lowering support for %assert as a value.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in adbb76c

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted in 381abce

(* assert(cond) via the %assert primitive — same semantics as the keyword form *)
let cond =
type_expect ~context:(Some AssertCondition) env scond Predef.type_bool
in
let exp_type =
match cond.exp_desc with
| Texp_construct (_, {cstr_name = "false"}, _) -> instance env ty_expected
| _ -> instance_def Predef.type_unit
in
rue
{
exp_desc = Texp_assert cond;
exp_loc = loc;
exp_extra = [];
exp_type;
exp_attributes = sexp.pexp_attributes;
exp_env = env;
}
| Pexp_apply {funct = sfunct; args = sargs; partial; transformed_jsx} ->
assert (sargs <> []);
begin_def ();
Expand Down Expand Up @@ -3337,24 +3359,6 @@ and type_expect_ ?deprecated_context ~context ?in_function ?(recarg = Rejected)
exp_attributes = sexp.pexp_attributes;
exp_env = env;
}
| Pexp_assert e ->
let cond =
type_expect ~context:(Some AssertCondition) env e Predef.type_bool
in
let exp_type =
match cond.exp_desc with
| Texp_construct (_, {cstr_name = "false"}, _) -> instance env ty_expected
| _ -> instance_def Predef.type_unit
in
rue
{
exp_desc = Texp_assert cond;
exp_loc = loc;
exp_extra = [];
exp_type;
exp_attributes = sexp.pexp_attributes;
exp_env = env;
}
| Pexp_newtype ({txt = name}, sbody) ->
let ty = newvar () in
(* remember original level *)
Expand Down
1 change: 0 additions & 1 deletion compiler/syntax/src/res_ast_debugger.ml
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,6 @@ module SexpAst = struct
extension_constructor ext_constr;
expression expr;
]
| Pexp_assert expr -> Sexp.list [Sexp.atom "Pexp_assert"; expression expr]
| Pexp_newtype (lbl, expr) ->
Sexp.list
[Sexp.atom "Pexp_newtype"; string lbl.Asttypes.txt; expression expr]
Expand Down
7 changes: 0 additions & 7 deletions compiler/syntax/src/res_comments_table.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1191,13 +1191,6 @@ and walk_expression expr t comments =
attach t.leading expr2.pexp_loc leading;
walk_expression expr2 t inside;
attach t.trailing expr2.pexp_loc trailing
| Pexp_assert expr ->
if is_block_expr expr then walk_expression expr t comments
else
let leading, inside, trailing = partition_by_loc comments expr.pexp_loc in
attach t.leading expr.pexp_loc leading;
walk_expression expr t inside;
attach t.trailing expr.pexp_loc trailing
| Pexp_coerce (expr, (), typexpr) ->
let leading, inside, trailing = partition_by_loc comments expr.pexp_loc in
attach t.leading expr.pexp_loc leading;
Expand Down
6 changes: 0 additions & 6 deletions compiler/syntax/src/res_core.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2374,15 +2374,9 @@ and parse_unary_expr p =
* If you have `a + b`, `a` and `b` both represent
* the operands of the binary expression with opeartor `+` *)
and parse_operand_expr ~context p =
let start_pos = p.Parser.start_pos in
let attrs = ref (parse_attributes p) in
let expr =
match p.Parser.token with
| Assert ->
Parser.next p;
let expr = parse_expr p in
let loc = mk_loc start_pos p.prev_end_pos in
Ast_helper.Exp.assert_ ~loc expr
| Lident "async"
(* we need to be careful when we're in a ternary true branch:
`condition ? ternary-true-branch : false-branch`
Expand Down
10 changes: 5 additions & 5 deletions compiler/syntax/src/res_grammar.ml
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,10 @@ let is_atomic_typ_expr_start = function
| _ -> false

let is_expr_start = function
| Token.Assert | At | Await | Backtick | Bang | Codepoint _ | False | Float _
| For | Hash | If | Int _ | Lbrace | Lbracket | LessThan | Lident _ | List
| Lparen | Minus | MinusDot | Module | Percent | Plus | PlusDot | Bnot | Bor
| Bxor | Band | String _ | Switch | True | Try | Uident _
| Token.At | Await | Backtick | Bang | Codepoint _ | False | Float _ | For
| Hash | If | Int _ | Lbrace | Lbracket | LessThan | Lident _ | List | Lparen
| Minus | MinusDot | Module | Percent | Plus | PlusDot | Bnot | Bor | Bxor
| Band | String _ | Switch | True | Try | Uident _
| Underscore (* _ => doThings() *)
| While | Forwardslash | ForwardslashDot | Dict ->
true
Expand Down Expand Up @@ -264,7 +264,7 @@ let is_attribute_start = function
let is_jsx_child_start = is_atomic_expr_start

let is_block_expr_start = function
| Token.Assert | At | Await | Backtick | Bang | Break | Codepoint _ | Continue
| Token.At | Await | Backtick | Bang | Break | Codepoint _ | Continue
| Exception | False | Float _ | For | Forwardslash | ForwardslashDot | Hash
| If | Int _ | Lbrace | Lbracket | LessThan | Let _ | Lident _ | List | Lparen
| Minus | MinusDot | Module | Open | Percent | Plus | PlusDot | String _
Expand Down
Loading
Loading