Skip to content

Commit 4fd6de4

Browse files
committed
clean up
1 parent cb5d0ca commit 4fd6de4

File tree

7 files changed

+97
-17
lines changed

7 files changed

+97
-17
lines changed

jscomp/core/destruct_exn.md

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
2+
3+
4+
Its essential is
5+
6+
```ocaml
7+
external destruct : 'b -> (exn -> 'a)
8+
```
9+
10+
However it does not prevent things like
11+
12+
```ocaml
13+
destruct v begin fun exn ->
14+
Js.log exn ;
15+
match exn with
16+
| ..
17+
| ..
18+
```
19+
20+
Here it forces us to answer whether `v` is exception or not,
21+
22+
while such syntax below does not need us answer `v` is exception
23+
or not, it just asks us to answer it matches a branch of exception or not which can be done in a sound way.
24+
25+
```ocaml
26+
match%exn v with
27+
| ..
28+
| ..
29+
```
30+
31+
However, we need make sure such cases not happen
32+
33+
```ocaml
34+
match%exn v with
35+
| e -> ...
36+
37+
```
38+
Or any vagous pattern which needs us to answer if
39+
it is an exception or not

jscomp/runtime/caml_exceptions.ml

+36-2
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,44 @@ let create (str : string) : Caml_builtin_exceptions.exception_block =
6363
v
6464

6565
external isUndefined : 'a -> bool = "#is_undef"
66-
(** It could be either customized exception or built in exception *)
67-
let isCamlException e =
66+
67+
68+
(**
69+
This function should never throw
70+
It could be either customized exception or built in exception
71+
Note due to that in OCaml extensible variants have the same
72+
runtime representation as exception, so we can not
73+
really tell the difference.
74+
75+
However, if we make a false alarm, classified extensible variant
76+
as exception, it will be OKAY for nested pattern match
77+
78+
{[
79+
match toExn x : exn option with
80+
| Some _
81+
-> Js.log "Could be an OCaml exception or an open variant"
82+
(* If it is an Open variant, it will never pattern match,
83+
This is Okay, since exception could never have exhaustive pattern match
84+
85+
*)
86+
| None -> Js.log "Not an OCaml exception for sure"
87+
]}
88+
89+
However, there is still something wrong, since if user write such code
90+
{[
91+
match toExn x with
92+
| Some _ -> (* assert it is indeed an exception *)
93+
(* This assertion is wrong, since it could be an open variant *)
94+
| None -> (* assert it is not an exception *)
95+
]}
96+
97+
This is not a problem in `try .. with` since the logic above is not expressible, see more design in [destruct_exn.md]
98+
*)
99+
let isCamlExceptionOrOpenVariant e =
68100
Obj.tag e = object_tag (* nullary exception *)
69101
||
70102
let slot = Obj.field e 0 in
71103
not (isUndefined slot) &&
72104
(Obj.tag slot = object_tag)
105+
106+

jscomp/runtime/caml_exceptions.mli

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,4 @@ val get_id : unit -> nativeint
3535

3636
val create : string -> Caml_builtin_exceptions.exception_block
3737

38-
val isCamlException : Obj.t -> bool
38+
val isCamlExceptionOrOpenVariant : Obj.t -> bool

jscomp/runtime/js_exn.ml

+9-8
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,18 @@
2323
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *)
2424

2525

26-
type t (* = *)
27-
(* < stack : string Js.undefined ; *)
28-
(* message : string Js.undefined ; *)
29-
(* name : string Js.undefined; *)
30-
(* fileName : string Js.undefined *)
31-
(* > Js.t *)
26+
type t
3227

33-
exception Error of t
28+
type exn += Error of t
3429

3530
external stack : t -> string option = ""
3631
[@@bs.get] [@@bs.return undefined_to_opt]
32+
external message : t -> string option = ""
33+
[@@bs.get] [@@bs.return undefined_to_opt]
34+
external name : t -> string option = ""
35+
[@@bs.get] [@@bs.return undefined_to_opt]
36+
external fileName : t -> string option = ""
37+
[@@bs.get] [@@bs.return undefined_to_opt]
3738

3839
(**
3940
{[
@@ -44,7 +45,7 @@ external stack : t -> string option = ""
4445
]}
4546
*)
4647
let internalToOCamlException (e : Obj.t) =
47-
if Caml_exceptions.isCamlException e then
48+
if Caml_exceptions.isCamlExceptionOrOpenVariant e then
4849
(Obj.magic e : exn)
4950
else Error (Obj.magic (e : Obj.t) : t)
5051

jscomp/runtime/js_exn.mli

+6
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ exception Error of t
3030

3131
external stack : t -> string option = ""
3232
[@@bs.get] [@@bs.return undefined_to_opt]
33+
external message : t -> string option = ""
34+
[@@bs.get] [@@bs.return undefined_to_opt]
35+
external name : t -> string option = ""
36+
[@@bs.get] [@@bs.return undefined_to_opt]
37+
external fileName : t -> string option = ""
38+
[@@bs.get] [@@bs.return undefined_to_opt]
3339

3440
(** Used by the compiler internally *)
3541
val internalToOCamlException : Obj.t -> exn

lib/js/caml_exceptions.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ function create(str) {
2424
return v;
2525
}
2626

27-
function isCamlException(e) {
27+
function isCamlExceptionOrOpenVariant(e) {
2828
if (e.tag === 248) {
2929
return /* true */1;
3030
} else {
@@ -37,8 +37,8 @@ function isCamlException(e) {
3737
}
3838
}
3939

40-
exports.caml_set_oo_id = caml_set_oo_id;
41-
exports.get_id = get_id;
42-
exports.create = create;
43-
exports.isCamlException = isCamlException;
40+
exports.caml_set_oo_id = caml_set_oo_id;
41+
exports.get_id = get_id;
42+
exports.create = create;
43+
exports.isCamlExceptionOrOpenVariant = isCamlExceptionOrOpenVariant;
4444
/* No side effect */

lib/js/js_exn.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ var Caml_exceptions = require("./caml_exceptions.js");
66
var $$Error = Caml_exceptions.create("Js_exn.Error");
77

88
function internalToOCamlException(e) {
9-
if (Caml_exceptions.isCamlException(e)) {
9+
if (Caml_exceptions.isCamlExceptionOrOpenVariant(e)) {
1010
return e;
1111
} else {
1212
return [

0 commit comments

Comments
 (0)