Skip to content

Commit dbf12b4

Browse files
bobzhangbobzhang
authored
Merge pull request #454 from bloomberg/check_mode_for_external_defs
add check mode to address #441
2 parents 85dba3d + 93618e1 commit dbf12b4

File tree

9 files changed

+232
-84
lines changed

9 files changed

+232
-84
lines changed

‎jscomp/core.mllib

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ ocaml_parse
1313
lam
1414
lam_iter
1515
lam_print
16+
lam_external_def
1617
lam_compile_env
1718
lam_dispatch_primitive
1819
lam_stats

‎jscomp/js_main.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ let buckle_script_flags =
7777
" set will generate `.d.ts` file for typescript (experimental)")
7878
:: ("-bs-diagnose", Arg.Set Js_config.diagnose,
7979
" More verbose output")
80-
:: ("-bs-rest-files", Arg.Rest collect_file,
80+
:: ("-bs-files", Arg.Rest collect_file,
8181
" Provide batch of files, the compiler will sort it before compiling"
8282
)
8383
:: Ocaml_options.mk_impl impl

‎jscomp/lam_compile_external_call.ml

Lines changed: 38 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -32,80 +32,36 @@
3232
module E = Js_exp_make
3333

3434

35+
(**
36+
[@@bs.module "react"]
37+
[@@bs.module "react"]
38+
---
39+
[@@bs.module "@" "react"]
40+
[@@bs.module "@" "react"]
41+
42+
They should have the same module name
43+
44+
TODO: we should emit an warning if we bind
45+
two external files to the same module name
46+
*)
3547

36-
type external_module_name =
37-
| Single of string
38-
| Bind of string * string
39-
40-
type 'a external_module = {
41-
txt : 'a ;
42-
external_module_name : external_module_name option;
43-
}
44-
45-
let handle_external module_name =
46-
begin
48+
let handle_external (module_name : Lam_external_def.external_module_name option) =
4749
match module_name with
48-
| Some module_name ->
49-
(*
50-
[@@bs.module "react"]
51-
[@@bs.module "react"]
52-
---
53-
[@@bs.module "@" "react"]
54-
[@@bs.module "@" "react"]
55-
They should have the same module name
56-
57-
TODO: we should emit an warning if we bind
58-
two external files to the same module name
59-
*)
50+
| Some {bundle ; bind_name} ->
6051
let id =
61-
match module_name with
62-
| Single module_name ->
63-
(Lam_compile_env.add_js_module module_name , module_name)
64-
| Bind (module_name, name) ->
65-
(Lam_compile_env.add_js_module
66-
~id:(Ext_ident.create_js_module name) module_name,
67-
module_name)
52+
match bind_name with
53+
| None ->
54+
Lam_compile_env.add_js_module bundle , bundle
55+
| Some bind_name ->
56+
Lam_compile_env.add_js_module
57+
~id:(Ext_ident.create_js_module bind_name) bundle,
58+
bundle
6859
in Some id
6960
| None -> None
70-
end
7161

72-
type js_call = {
73-
splice : bool ;
74-
qualifiers : string list;
75-
name : string;
76-
}
77-
78-
type js_send = {
79-
splice : bool ;
80-
name : string
81-
} (* we know it is a js send, but what will happen if you pass an ocaml objct *)
82-
83-
type js_val = {
84-
name : string ;
85-
external_module_name : external_module_name option;
86-
87-
}
88-
89-
type js_new = { name : string }
90-
type js_set = { name : string }
91-
type js_get = { name : string }
92-
93-
type ffi =
94-
| Obj_create
95-
| Js_global of js_val
96-
| Js_global_as_var of external_module_name
97-
| Js_call of js_call external_module
98-
| Js_send of js_send
99-
| Js_new of js_new external_module
100-
| Js_set of js_set
101-
| Js_get of js_get
102-
| Js_get_index
103-
| Js_set_index
104-
| Normal
105-
(* When it's normal, it is handled as normal c functional ffi call *)
106-
type prim = Types.type_expr option Primitive.description
107-
108-
let handle_attributes ({prim_attributes ; prim_name} as _prim : prim ) : Location.t option * ffi =
62+
63+
let handle_attributes ({prim_attributes ; prim_name} as _prim : Lam_external_def.prim )
64+
: Location.t option * Lam_external_def.ffi =
10965
let qualifiers = ref [] in
11066
let call_name = ref None in
11167
let external_module_name = ref None in
@@ -145,19 +101,15 @@ let handle_attributes ({prim_attributes ; prim_name} as _prim : prim ) : Locati
145101
| Some name ->
146102
js_val := `Value name
147103
| None ->
148-
js_val := `Value _prim.prim_name
104+
js_val := `Value prim_name
149105
(* we can report error here ... *)
150106
end
151107
| "bs.val_of_module"
152108
(* {[ [@@bs.val_of_module]]}
153109
*)
154110
->
155-
begin match Ast_payload.is_single_string pay_load with
156-
| Some name ->
157-
js_val_of_module := `Value(Bind (name, prim_name))
158-
| None ->
159-
js_val_of_module := `Value (Single prim_name)
160-
end
111+
js_val_of_module :=
112+
`Value (Lam_external_def.{bundle = prim_name ; bind_name = Ast_payload.is_single_string pay_load})
161113
|"bs.splice"
162114
->
163115
js_splice := true
@@ -166,19 +118,19 @@ let handle_attributes ({prim_attributes ; prim_name} as _prim : prim ) : Locati
166118
->
167119
begin match Ast_payload.is_single_string pay_load with
168120
| Some name -> js_send := `Value name
169-
| None -> js_send := `Value _prim.prim_name
121+
| None -> js_send := `Value prim_name
170122
end
171123
| "bs.set"
172124
->
173125
begin match Ast_payload.is_single_string pay_load with
174126
| Some name -> js_set := `Value name
175-
| None -> js_set := `Value _prim.prim_name
127+
| None -> js_set := `Value prim_name
176128
end
177129
| "bs.get"
178130
->
179131
begin match Ast_payload.is_single_string pay_load with
180132
| Some name -> js_get := `Value name
181-
| None -> js_get := `Value _prim.prim_name
133+
| None -> js_get := `Value prim_name
182134
end
183135

184136
| "bs.call"
@@ -188,20 +140,23 @@ let handle_attributes ({prim_attributes ; prim_name} as _prim : prim ) : Locati
188140
->
189141
begin match Ast_payload.is_single_string pay_load with
190142
| Some name -> call_name := Some (x.loc, name)
191-
| None -> call_name := Some(x.loc, _prim.prim_name)
143+
| None -> call_name := Some(x.loc, prim_name)
192144
end
193145
| "bs.module" ->
194146
begin match Ast_payload.is_string_or_strings pay_load with
195-
| `Single name -> external_module_name:= Some (Single name)
196-
| `Some [a;b] -> external_module_name := Some (Bind (a,b))
147+
| `Single name ->
148+
external_module_name:= Some (Lam_external_def.{ bundle = name; bind_name = None})
149+
| `Some [bundle;bind_name] ->
150+
external_module_name :=
151+
Some (Lam_external_def.{bundle ; bind_name = Some bind_name})
197152
| `Some _ -> ()
198153
| `None -> () (* should emit a warning instead *)
199154
end
200155

201156
| "bs.new" ->
202157
begin match Ast_payload.is_single_string pay_load with
203158
| Some x -> js_new := Some x
204-
| None -> js_new := Some _prim.prim_name
159+
| None -> js_new := Some prim_name
205160
end
206161
| "bs.set_index"
207162
-> js_set_index := true
@@ -324,6 +279,7 @@ let translate
324279
(args : J.expression list) =
325280
begin
326281
let loc, ffi = handle_attributes prim in
282+
let () = Lam_external_def.check_ffi ?loc ffi in
327283
match ffi with
328284
| Obj_create ->
329285
begin

‎jscomp/lam_external_def.ml

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
(* Copyright (C) 2015-2016 Bloomberg Finance L.P.
2+
*
3+
* This program is free software: you can redistribute it and/or modify
4+
* it under the terms of the GNU Lesser General Public License as published by
5+
* the Free Software Foundation, either version 3 of the License, or
6+
* (at your option) any later version.
7+
*
8+
* In addition to the permissions granted to you by the LGPL, you may combine
9+
* or link a "work that uses the Library" with a publicly distributed version
10+
* of this file to produce a combined library or application, then distribute
11+
* that combined work under the terms of your choosing, with no requirement
12+
* to comply with the obligations normally placed on you by section 4 of the
13+
* LGPL version 3 (or the corresponding section of a later version of the LGPL
14+
* should you choose to use a later version).
15+
*
16+
* This program is distributed in the hope that it will be useful,
17+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
18+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
19+
* GNU Lesser General Public License for more details.
20+
*
21+
* You should have received a copy of the GNU Lesser General Public License
22+
* along with this program; if not, write to the Free Software
23+
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *)
24+
25+
26+
27+
type external_module_name =
28+
{ bundle : string ;
29+
bind_name : string option
30+
}
31+
type 'a external_module = {
32+
txt : 'a ;
33+
external_module_name : external_module_name option;
34+
}
35+
36+
37+
type js_call = {
38+
splice : bool ;
39+
qualifiers : string list;
40+
name : string;
41+
}
42+
43+
type js_send = {
44+
splice : bool ;
45+
name : string
46+
} (* we know it is a js send, but what will happen if you pass an ocaml objct *)
47+
48+
type js_val = {
49+
name : string ;
50+
external_module_name : external_module_name option;
51+
52+
}
53+
54+
type js_new = { name : string }
55+
type js_set = { name : string }
56+
type js_get = { name : string }
57+
58+
type ffi =
59+
| Obj_create
60+
| Js_global of js_val
61+
| Js_global_as_var of external_module_name
62+
| Js_call of js_call external_module
63+
| Js_send of js_send
64+
| Js_new of js_new external_module
65+
| Js_set of js_set
66+
| Js_get of js_get
67+
| Js_get_index
68+
| Js_set_index
69+
| Normal
70+
(* When it's normal, it is handled as normal c functional ffi call *)
71+
type prim = Types.type_expr option Primitive.description
72+
73+
let check_external_module_name ?loc x =
74+
match x with
75+
| {bundle = ""; _ } | {bind_name = Some ""} ->
76+
Location.raise_errorf ?loc "empty name encountered"
77+
| _ -> ()
78+
let check_external_module_name_opt ?loc x =
79+
match x with
80+
| None -> ()
81+
| Some v -> check_external_module_name ?loc v
82+
83+
84+
let check_ffi ?loc ffi =
85+
match ffi with
86+
| Js_global {name = ""}
87+
| Js_send {name = ""}
88+
| Js_set {name = ""}
89+
| Js_get {name = ""}
90+
-> Location.raise_errorf ?loc "empty name encountered"
91+
| Js_global _ | Js_send _ | Js_set _ | Js_get _
92+
| Obj_create
93+
| Js_get_index | Js_set_index | Normal -> ()
94+
95+
| Js_global_as_var external_module_name
96+
-> check_external_module_name external_module_name
97+
| Js_new {external_module_name ; txt = {name ; _}}
98+
| Js_call {external_module_name ; txt = {name ; _}}
99+
->
100+
check_external_module_name_opt ?loc external_module_name ;
101+
if name = "" then
102+
Location.raise_errorf ?loc "empty name in externals"

‎jscomp/lam_external_def.mli

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
(* Copyright (C) 2015-2016 Bloomberg Finance L.P.
2+
*
3+
* This program is free software: you can redistribute it and/or modify
4+
* it under the terms of the GNU Lesser General Public License as published by
5+
* the Free Software Foundation, either version 3 of the License, or
6+
* (at your option) any later version.
7+
*
8+
* In addition to the permissions granted to you by the LGPL, you may combine
9+
* or link a "work that uses the Library" with a publicly distributed version
10+
* of this file to produce a combined library or application, then distribute
11+
* that combined work under the terms of your choosing, with no requirement
12+
* to comply with the obligations normally placed on you by section 4 of the
13+
* LGPL version 3 (or the corresponding section of a later version of the LGPL
14+
* should you choose to use a later version).
15+
*
16+
* This program is distributed in the hope that it will be useful,
17+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
18+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
19+
* GNU Lesser General Public License for more details.
20+
*
21+
* You should have received a copy of the GNU Lesser General Public License
22+
* along with this program; if not, write to the Free Software
23+
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *)
24+
25+
26+
type external_module_name =
27+
{ bundle : string ;
28+
bind_name : string option
29+
}
30+
type 'a external_module = {
31+
txt : 'a ;
32+
external_module_name : external_module_name option;
33+
}
34+
35+
36+
type js_call = {
37+
splice : bool ;
38+
qualifiers : string list;
39+
name : string;
40+
}
41+
42+
type js_send = {
43+
splice : bool ;
44+
name : string
45+
} (* we know it is a js send, but what will happen if you pass an ocaml objct *)
46+
47+
type js_val = {
48+
name : string ;
49+
external_module_name : external_module_name option;
50+
51+
}
52+
53+
type js_new = { name : string }
54+
type js_set = { name : string }
55+
type js_get = { name : string }
56+
57+
type ffi =
58+
| Obj_create
59+
| Js_global of js_val
60+
| Js_global_as_var of external_module_name
61+
| Js_call of js_call external_module
62+
| Js_send of js_send
63+
| Js_new of js_new external_module
64+
| Js_set of js_set
65+
| Js_get of js_get
66+
| Js_get_index
67+
| Js_set_index
68+
| Normal
69+
(* When it's normal, it is handled as normal c functional ffi call *)
70+
type prim = Types.type_expr option Primitive.description
71+
72+
val check_ffi : ?loc:Location.t -> ffi -> unit

‎jscomp/test/.depend

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,8 @@ google_closure_test.cmj : test_google_closure.cmj mt.cmi
222222
google_closure_test.cmx : test_google_closure.cmx mt.cmx
223223
gpr_405_test.cmj : ../stdlib/hashtbl.cmi gpr_405_test.cmi
224224
gpr_405_test.cmx : ../stdlib/hashtbl.cmx gpr_405_test.cmi
225+
gpr_441.cmj :
226+
gpr_441.cmx :
225227
guide_for_ext.cmj :
226228
guide_for_ext.cmx :
227229
hamming_test.cmj : ../stdlib/printf.cmi mt.cmi ../stdlib/lazy.cmi \
@@ -886,6 +888,8 @@ google_closure_test.cmo : test_google_closure.cmo mt.cmi
886888
google_closure_test.cmj : test_google_closure.cmj mt.cmj
887889
gpr_405_test.cmo : ../stdlib/hashtbl.cmi gpr_405_test.cmi
888890
gpr_405_test.cmj : ../stdlib/hashtbl.cmj gpr_405_test.cmi
891+
gpr_441.cmo :
892+
gpr_441.cmj :
889893
guide_for_ext.cmo :
890894
guide_for_ext.cmj :
891895
hamming_test.cmo : ../stdlib/printf.cmi mt.cmi ../stdlib/lazy.cmi \

0 commit comments

Comments
 (0)