Skip to content

Commit 12e13dc

Browse files
authored
Merge pull request #3816 from BuckleScript/clean_up
clean up && make is_pure_module forgiving when not found
2 parents 5a10d0e + 50763e5 commit 12e13dc

File tree

10 files changed

+221
-340
lines changed

10 files changed

+221
-340
lines changed

jscomp/core/js_cmj_format.ml

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -67,21 +67,7 @@ let cmj_magic_number = "BUCKLE20171012"
6767
let cmj_magic_number_length =
6868
String.length cmj_magic_number
6969

70-
let pure_dummy =
71-
{
72-
values = empty_values;
73-
pure = true;
74-
npm_package_path = Js_packages_info.empty;
75-
cmj_case = Little_js;
76-
}
7770

78-
let no_pure_dummy =
79-
{
80-
values = empty_values;
81-
pure = false;
82-
npm_package_path = Js_packages_info.empty;
83-
cmj_case = Little_js; (** TODO: consistent with Js_config.bs_suffix default *)
84-
}
8571

8672
let digest_length = 16 (*16 chars *)
8773

jscomp/core/js_cmj_format.mli

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,7 @@ val get_cmj_case :
9393
cmj_case
9494

9595
val single_na : arity
96-
val pure_dummy : t
97-
val no_pure_dummy : t
96+
9897

9998

10099
val from_file : string -> t

jscomp/core/js_cmj_load.ml

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,14 @@
2828
*)
2929

3030

31+
type path = string
32+
type cmj_load_info = {
33+
cmj_table : Js_cmj_format.t ;
34+
cmj_path : path ;
35+
}
3136

3237
#if BS_COMPILER_IN_BROWSER then
33-
let find_cmj_exn file : string * Js_cmj_format.t =
38+
let find_cmj_exn file : cmj_load_info =
3439
let target = Ext_string.uncapitalize_ascii (Filename.basename file) in
3540
match String_map.find_exn !Js_cmj_datasets.data_sets target with
3641
| v
@@ -41,18 +46,18 @@ let find_cmj_exn file : string * Js_cmj_format.t =
4146
Ext_log.warn __LOC__
4247
"@[%s corrupted in database, when looking %s while compiling %s please update @]" file target !Location.input_name ;
4348
Bs_exception.error (Cmj_not_found file)
44-
| v -> "BROWSER", v
49+
| v -> {cmj_path = "BROWSER"; cmj_table = v}
4550
(* see {!Js_packages_info.string_of_module_id} *)
4651
end
4752
| exception Not_found
4853
->
4954
Bs_exception.error (Cmj_not_found file)
5055
#else
51-
let find_cmj_exn file : string * Js_cmj_format.t =
56+
let find_cmj_exn file : cmj_load_info =
5257
match Config_util.find_opt file with
5358
| Some f
5459
->
55-
f, Js_cmj_format.from_file f
60+
{cmj_path = f; cmj_table = Js_cmj_format.from_file f}
5661
| None ->
5762
(* ONLY read the stored cmj data in browser environment *)
5863
Bs_exception.error (Cmj_not_found file)

jscomp/core/js_cmj_load.mli

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,13 @@
2727
it depends on {!Js_cmj_datasets}, for non-browser environment, it fails
2828
*)
2929

30-
30+
type path = string
31+
type cmj_load_info = {
32+
cmj_table : Js_cmj_format.t ;
33+
cmj_path : path ;
34+
}
3135

3236
(** return path and meta data *)
33-
val find_cmj_exn : string -> string * Js_cmj_format.t
37+
val find_cmj_exn :
38+
string ->
39+
cmj_load_info

jscomp/core/js_fold.ml

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,10 +400,17 @@ class virtual fold =
400400
{[ goto : label option ; ]}
401401
*)
402402
'a. ('self_type -> 'a -> 'self_type) -> 'a case_clause -> 'self_type =
403-
fun _f_a { switch_case = _x; switch_body = _x_i1; should_break = _x_i2
403+
fun _f_a
404+
{
405+
switch_case = _x;
406+
switch_body = _x_i1;
407+
should_break = _x_i2;
408+
comment = _x_i3
404409
} ->
405410
let o = _f_a o _x in
406-
let o = o#block _x_i1 in let o = o#bool _x_i2 in o
411+
let o = o#block _x_i1 in
412+
let o = o#bool _x_i2 in
413+
let o = o#option (fun o -> o#string) _x_i3 in o
407414
method block : block -> 'self_type = (* true means break *)
408415
(* TODO: For efficency: block should not be a list, it should be able to
409416
be concatenated in both ways

jscomp/core/js_implementation.ml

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,14 @@ let fprintf = Format.fprintf
2222

2323

2424

25-
let print_if ppf flag printer arg =
25+
let print_if_pipe ppf flag printer arg =
2626
if !flag then fprintf ppf "%a@." printer arg;
2727
arg
2828

29+
let print_if ppf flag printer arg =
30+
if !flag then fprintf ppf "%a@." printer arg
31+
32+
2933

3034
let process_with_gentype filename =
3135
match !Clflags.bs_gentype with
@@ -103,15 +107,15 @@ let interface ppf fname outputprefix =
103107
Compmisc.init_path false;
104108
Pparse.parse_interface ~tool_name:Js_config.tool_name ppf fname
105109
|> Ppx_entry.rewrite_signature
106-
|> print_if ppf Clflags.dump_parsetree Printast.interface
107-
|> print_if ppf Clflags.dump_source Pprintast.signature
110+
|> print_if_pipe ppf Clflags.dump_parsetree Printast.interface
111+
|> print_if_pipe ppf Clflags.dump_source Pprintast.signature
108112
|> after_parsing_sig ppf outputprefix
109113

110114
let interface_mliast ppf fname outputprefix =
111115
Compmisc.init_path false;
112116
Binary_ast.read_ast Mli fname
113-
|> print_if ppf Clflags.dump_parsetree Printast.interface
114-
|> print_if ppf Clflags.dump_source Pprintast.signature
117+
|> print_if_pipe ppf Clflags.dump_parsetree Printast.interface
118+
|> print_if_pipe ppf Clflags.dump_source Pprintast.signature
115119
|> after_parsing_sig ppf outputprefix
116120

117121

@@ -140,43 +144,41 @@ let after_parsing_impl ppf outputprefix ast =
140144
Lam_compile_env.reset () ;
141145
let env = Compmisc.initial_env() in
142146
Env.set_unit_name modulename;
143-
144147
let (typedtree, coercion, _, _) =
145-
ast
146-
|> Typemod.type_implementation_more ?check_exists:(if !Js_config.force_cmi then None else Some ()) !Location.input_name outputprefix modulename env
147-
|> print_if ppf Clflags.dump_typedtree
148-
(fun fmt (ty,co,_,_) -> Printtyped.implementation_with_coercion fmt (ty,co))
149-
in
148+
Typemod.type_implementation_more
149+
?check_exists:(if !Js_config.force_cmi then None else Some ())
150+
!Location.input_name outputprefix modulename env ast in
151+
let typedtree_coercion = (typedtree, coercion) in
152+
print_if ppf Clflags.dump_typedtree
153+
Printtyped.implementation_with_coercion typedtree_coercion ;
150154
if !Clflags.print_types || !Js_config.cmi_only then begin
151155
Warnings.check_fatal ();
152156
end else begin
153-
(typedtree, coercion)
154-
|> Translmod.transl_implementation modulename
155-
|> (fun lambda ->
156-
let js_program =
157-
print_if ppf Clflags.dump_rawlambda Printlambda.lambda (get_lambda lambda)
158-
|> Lam_compile_main.compile outputprefix in
159-
if not !Js_config.cmj_only then
160-
Lam_compile_main.lambda_as_module
161-
js_program
162-
outputprefix
163-
);
157+
let lambda = Translmod.transl_implementation modulename typedtree_coercion in
158+
let js_program =
159+
print_if_pipe ppf Clflags.dump_rawlambda Printlambda.lambda (get_lambda lambda)
160+
|> Lam_compile_main.compile outputprefix in
161+
if not !Js_config.cmj_only then
162+
Lam_compile_main.lambda_as_module
163+
js_program
164+
outputprefix
165+
;
164166
end;
165167
process_with_gentype (outputprefix ^ ".cmt")
166168
end
167169
let implementation ppf fname outputprefix =
168170
Compmisc.init_path false;
169171
Pparse.parse_implementation ~tool_name:Js_config.tool_name ppf fname
170172
|> Ppx_entry.rewrite_implementation
171-
|> print_if ppf Clflags.dump_parsetree Printast.implementation
172-
|> print_if ppf Clflags.dump_source Pprintast.structure
173+
|> print_if_pipe ppf Clflags.dump_parsetree Printast.implementation
174+
|> print_if_pipe ppf Clflags.dump_source Pprintast.structure
173175
|> after_parsing_impl ppf outputprefix
174176

175177
let implementation_mlast ppf fname outputprefix =
176178
Compmisc.init_path false;
177179
Binary_ast.read_ast Ml fname
178-
|> print_if ppf Clflags.dump_parsetree Printast.implementation
179-
|> print_if ppf Clflags.dump_source Pprintast.structure
180+
|> print_if_pipe ppf Clflags.dump_parsetree Printast.implementation
181+
|> print_if_pipe ppf Clflags.dump_source Pprintast.structure
180182
|> after_parsing_impl ppf outputprefix
181183

182184

@@ -212,7 +214,7 @@ let implementation_map ppf sourcefile outputprefix =
212214
) in
213215
Compmisc.init_path false;
214216
ml_ast
215-
|> print_if ppf Clflags.dump_parsetree Printast.implementation
216-
|> print_if ppf Clflags.dump_source Pprintast.structure
217+
|> print_if_pipe ppf Clflags.dump_parsetree Printast.implementation
218+
|> print_if_pipe ppf Clflags.dump_source Pprintast.structure
217219
|> after_parsing_impl ppf outputprefix
218220

jscomp/core/lam_compile_env.ml

Lines changed: 28 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,10 @@
3232
module E = Js_exp_make
3333
module S = Js_stmt_make
3434

35-
type path = string
36-
37-
type ml_module_info = {
38-
cmj_table : Js_cmj_format.t ;
39-
cmj_path : path;
40-
}
4135

4236
type env_value =
43-
| Ml of ml_module_info
44-
| Runtime of ml_module_info
37+
| Ml of Js_cmj_load.cmj_load_info
38+
| Runtime of Js_cmj_load.cmj_load_info
4539
(**
4640
[Runtime (pure, path, cmj_format)]
4741
A built in module probably from our runtime primitives,
@@ -124,10 +118,10 @@ let query_external_id_info (module_id : Ident.t) (name : string) : ident_info =
124118
let cmj_table =
125119
match Lam_module_ident.Hash.find_opt cached_tbl oid with
126120
| None ->
127-
let cmj_path, cmj_table =
121+
let cmj_load_info =
128122
Js_cmj_load.find_cmj_exn (module_id.name ^ Literals.suffix_cmj) in
129-
oid +> Ml { cmj_table ; cmj_path } ;
130-
cmj_table
123+
oid +> Ml cmj_load_info ;
124+
cmj_load_info.cmj_table
131125
| Some (Ml { cmj_table } )
132126
-> cmj_table
133127
| Some (Runtime _) -> assert false
@@ -145,50 +139,9 @@ let query_external_id_info (module_id : Ident.t) (name : string) : ident_info =
145139

146140

147141

148-
(* TODO: it does not make sense to cache
149-
[Runtime]
150-
and [externals]*)
151-
type _ t =
152-
| No_env : bool t
153-
| Has_env : Env.t -> bool t (* Indicate it is pure or not *)
154142

155143

156-
(*
157-
FIXME:
158-
Here [not_found] only means cmi not found, not cmj not found
159-
We do need handle cases when [not_found] hit in a graceful way
160-
*)
161-
let query_and_add_if_not_exist
162-
(type u)
163-
(oid : Lam_module_ident.t)
164-
=
165-
match Lam_module_ident.Hash.find_opt cached_tbl oid with
166-
| None ->
167-
begin match oid.kind with
168-
| Runtime ->
169-
let (cmj_path, cmj_table) as cmj_info =
170-
Js_cmj_load.find_cmj_exn (Lam_module_ident.name oid ^ Literals.suffix_cmj) in
171-
oid +> Runtime {cmj_path;cmj_table} ;
172-
Js_cmj_format.is_pure cmj_table
173-
| Ml
174-
->
175-
let (cmj_path, cmj_table) as cmj_info =
176-
Js_cmj_load.find_cmj_exn (Lam_module_ident.name oid ^ Literals.suffix_cmj) in
177-
oid +> Ml {cmj_table;cmj_path } ;
178-
Js_cmj_format.is_pure cmj_table
179-
| External _ ->
180-
oid +> External;
181-
(** This might be wrong, if we happen to expand an js module
182-
we should assert false (but this in general should not happen)
183-
FIXME: #154, it come from External, should be okay
184-
*)
185-
false
186-
end
187-
| Some (Ml { cmj_table })
188-
| Some (Runtime {cmj_table}) ->
189-
Js_cmj_format.is_pure cmj_table
190-
| Some External -> false
191-
144+
192145

193146

194147

@@ -213,21 +166,37 @@ let get_package_path_from_cmj
213166
| Runtime
214167
| External _ -> assert false
215168
| Ml ->
216-
let (cmj_path, cmj_table) =
169+
let ({Js_cmj_load.cmj_table} as cmj_load_info) =
217170
Js_cmj_load.find_cmj_exn (Lam_module_ident.name id ^ Literals.suffix_cmj) in
218-
id +> Ml {cmj_table;cmj_path };
219-
(cmj_path,
171+
id +> Ml cmj_load_info;
172+
(cmj_load_info.cmj_path,
220173
Js_cmj_format.get_npm_package_path cmj_table,
221174
Js_cmj_format.get_cmj_case cmj_table )
222175
end
223176

224177
let add = Lam_module_ident.Hash_set.add
225178

226179

180+
227181
(* Conservative interface *)
228-
let is_pure_module (id : Lam_module_ident.t) =
229-
id.kind = Runtime ||
230-
query_and_add_if_not_exist id
182+
let is_pure_module (oid : Lam_module_ident.t) =
183+
match oid.kind with
184+
| Runtime -> true
185+
| External _ -> false
186+
| Ml ->
187+
begin match Lam_module_ident.Hash.find_opt cached_tbl oid with
188+
| None ->
189+
begin
190+
match Js_cmj_load.find_cmj_exn (Lam_module_ident.name oid ^ Literals.suffix_cmj) with
191+
| cmj_load_info ->
192+
oid +> Ml cmj_load_info ;
193+
Js_cmj_format.is_pure cmj_load_info.cmj_table
194+
| exception _ -> false
195+
end
196+
| Some (Ml{cmj_table}|Runtime {cmj_table}) ->
197+
Js_cmj_format.is_pure cmj_table
198+
| Some External -> false
199+
end
231200

232201

233202
let get_required_modules

jscomp/core/lam_compile_env.mli

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,23 +24,10 @@
2424

2525

2626

27-
28-
29-
30-
31-
3227
(** Helper for global Ocaml module index into meaningful names *)
3328

3429

3530

36-
type path = string
37-
38-
39-
40-
41-
type _ t =
42-
| No_env : bool t
43-
| Has_env : Env.t -> bool t
4431

4532

4633
type ident_info = {

0 commit comments

Comments
 (0)