-
Notifications
You must be signed in to change notification settings - Fork 484
Remove assert as a reserved keyword
#8399
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
base: master
Are you sure you want to change the base?
Changes from all commits
8cc75fe
2c71636
adbb76c
3548f36
d22712c
842e05d
14b5423
0cbe0e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, _, _) | ||
|
|
@@ -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
+2505
to
+2509
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This special-case only rewrites direct Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in adbb76c |
||
| (* 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 (); | ||
|
|
@@ -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 *) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-noassertbehavior for first-classassertWhen
%assertis 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 (directassert(...)is lowered to()without evaluating the condition), so enabling-noassertno longer reliably removes assertion-condition side effects onceassertis passed around as a value.Useful? React with 👍 / 👎.