Skip to content

Commit b641a35

Browse files
authored
Refactor Persistent_env to separate imports from bound names (#1764)
With the advent of parameters, we have reasons to refer to global module names besides those names being runtime global variables, or addressable values at all. In addition, we're going to want to load a .cmi but then parameterise it in different ways to get different signatures, each of which will be bound to a separate value. In the end, then, instead of just `pers_struct`, we'll want three different record types, each with an associated cache: * `import`, corresponding directly to a .cmi file, keyed by `Compilation_unit.Name.t` * `pers_name`, corresponding to an `import` with parameters applied to it, keyed by a `Compilation_unit.Name.t` and some parameters * `pers_struct`, corresponding to a persistent name that is actually bound in the environment, with the same key as `pers_name` For this PR, I'm leaving out the second cache as nothing about it is relevant yet, but the split is disruptive enough that even the two are worthwhile. Note that `pers_struct` is currently still keyed by `Compilation_unit.Name.t` since we don't yet have the datatypes for parameterised names. Besides the internal reorganisation of `Persistent_env`, this also changes the API between it and `Env`, offloading some of the work done by the callback, `read_sign_of_cmi`. Significantly, `read_sign_of_cmi` is no longer called _at all_ until something is going to be bound (i.e., become a `pers_struct`). This keeps the `Env.t` completely free of anything it shouldn't know about.
1 parent 2adb8bd commit b641a35

File tree

6 files changed

+207
-110
lines changed

6 files changed

+207
-110
lines changed

ocaml/.depend

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1305,35 +1305,43 @@ typing/patterns.cmi : \
13051305
parsing/asttypes.cmi
13061306
typing/persistent_env.cmo : \
13071307
utils/warnings.cmi \
1308+
typing/subst.cmi \
1309+
typing/shape.cmi \
13081310
utils/misc.cmi \
13091311
parsing/location.cmi \
13101312
utils/load_path.cmi \
13111313
utils/lazy_backtrack.cmi \
13121314
utils/import_info.cmi \
1315+
typing/ident.cmi \
13131316
utils/consistbl.cmi \
13141317
utils/compilation_unit.cmi \
13151318
file_formats/cmi_format.cmi \
13161319
utils/clflags.cmi \
13171320
typing/persistent_env.cmi
13181321
typing/persistent_env.cmx : \
13191322
utils/warnings.cmx \
1323+
typing/subst.cmx \
1324+
typing/shape.cmx \
13201325
utils/misc.cmx \
13211326
parsing/location.cmx \
13221327
utils/load_path.cmx \
13231328
utils/lazy_backtrack.cmx \
13241329
utils/import_info.cmx \
1330+
typing/ident.cmx \
13251331
utils/consistbl.cmx \
13261332
utils/compilation_unit.cmx \
13271333
file_formats/cmi_format.cmx \
13281334
utils/clflags.cmx \
13291335
typing/persistent_env.cmi
13301336
typing/persistent_env.cmi : \
13311337
typing/subst.cmi \
1338+
typing/shape.cmi \
13321339
utils/misc.cmi \
13331340
parsing/location.cmi \
13341341
utils/load_path.cmi \
13351342
utils/lazy_backtrack.cmi \
13361343
utils/import_info.cmi \
1344+
typing/ident.cmi \
13371345
utils/consistbl.cmi \
13381346
utils/compilation_unit.cmi \
13391347
file_formats/cmi_format.cmi

ocaml/testsuite/tests/templates/basic/bad_param_impl.ml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99
ocamlc_byte_exit_status = "2";
1010
compiler_output = "bad_param_impl.output";
1111
ocamlc.byte;
12-
reason = "error broken, will be fixed by #1764";
13-
skip;
1412
compiler_reference = "bad_param_impl.reference";
1513
check-ocamlc.byte-output;
1614
*)

ocaml/testsuite/tests/templates/basic/test.ml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
compiler_output = "bad_ref_direct.output";
99
ocamlc_byte_exit_status = "2";
1010
ocamlc.byte;
11-
reason = "correct error message not yet implemented";
12-
skip;
1311
compiler_reference = "bad_ref_direct.reference";
1412
check-ocamlc.byte-output;
1513
*)

ocaml/typing/env.ml

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ let map_summary f = function
173173
| Env_value_unbound (s, u, r) -> Env_value_unbound (f s, u, r)
174174
| Env_module_unbound (s, u, r) -> Env_module_unbound (f s, u, r)
175175

176-
type address =
176+
type address = Persistent_env.address =
177177
| Aunit of Compilation_unit.t
178178
| Alocal of Ident.t
179179
| Adot of address * int
@@ -953,34 +953,24 @@ let components_of_module ~alerts ~uid env ps path addr mty shape =
953953
}
954954
}
955955

956-
let read_sign_of_cmi { Persistent_env.Persistent_signature.cmi; _ } =
957-
let name =
958-
match cmi.cmi_kind with
959-
| Normal { cmi_impl } -> cmi_impl
960-
| Parameter -> Misc.fatal_error "Unsupported import of parameter module"
961-
in
962-
let sign = cmi.cmi_sign in
963-
let flags = cmi.cmi_flags in
964-
let id = Ident.create_persistent (Compilation_unit.name_as_string name) in
956+
let read_sign_of_cmi sign name uid ~shape ~address:addr ~flags =
957+
let id = Ident.create_persistent (Compilation_unit.Name.to_string name) in
965958
let path = Pident id in
966959
let alerts =
967960
List.fold_left (fun acc -> function Alerts s -> s | _ -> acc)
968961
Misc.Stdlib.String.Map.empty
969962
flags
970963
in
971-
let sign = Subst.Lazy.signature Make_local Subst.identity sign in
972964
let md =
973965
{ Subst.Lazy.md_type = Mty_signature sign;
974966
md_loc = Location.none;
975967
md_attributes = [];
976-
md_uid = Uid.of_compilation_unit_id name;
968+
md_uid = uid;
977969
}
978970
in
979-
let mda_address = Lazy_backtrack.create_forced (Aunit name) in
971+
let mda_address = Lazy_backtrack.create_forced addr in
980972
let mda_declaration = md in
981-
let mda_shape =
982-
Shape.for_persistent_unit (name |> Compilation_unit.full_path_as_string)
983-
in
973+
let mda_shape = shape in
984974
let mda_components =
985975
let mty = md.md_type in
986976
components_of_module ~alerts ~uid:md.md_uid
@@ -1016,7 +1006,7 @@ let check_pers_mod ~loc name =
10161006
Persistent_env.check !persistent_env read_sign_of_cmi ~loc name
10171007

10181008
let crc_of_unit name =
1019-
Persistent_env.crc_of_unit !persistent_env read_sign_of_cmi name
1009+
Persistent_env.crc_of_unit !persistent_env name
10201010

10211011
let is_imported_opaque modname =
10221012
Persistent_env.is_imported_opaque !persistent_env modname
@@ -2646,13 +2636,8 @@ let open_signature
26462636

26472637
(* Read a signature from a file *)
26482638
let read_signature modname filename ~add_binding =
2649-
let mda =
2650-
read_pers_mod modname filename ~add_binding
2651-
in
2652-
let md = Subst.Lazy.force_module_decl mda.mda_declaration in
2653-
match md.md_type with
2654-
| Mty_signature sg -> sg
2655-
| Mty_ident _ | Mty_functor _ | Mty_alias _ | Mty_strengthen _ -> assert false
2639+
let mty = read_pers_mod modname filename ~add_binding in
2640+
Subst.Lazy.force_signature mty
26562641

26572642
let is_identchar_latin1 = function
26582643
| 'A'..'Z' | 'a'..'z' | '_' | '\192'..'\214' | '\216'..'\246'

0 commit comments

Comments
 (0)