Skip to content

Commit

Permalink
flambda-backend: Refactor Persistent_env to separate imports from b…
Browse files Browse the repository at this point in the history
…ound names (#2585)

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.
  • Loading branch information
lukemaurer authored May 15, 2024
1 parent 3626027 commit ca99fbd
Show file tree
Hide file tree
Showing 6 changed files with 207 additions and 110 deletions.
8 changes: 8 additions & 0 deletions .depend
Original file line number Diff line number Diff line change
Expand Up @@ -1321,35 +1321,43 @@ typing/patterns.cmi : \
parsing/asttypes.cmi
typing/persistent_env.cmo : \
utils/warnings.cmi \
typing/subst.cmi \
typing/shape.cmi \
utils/misc.cmi \
parsing/location.cmi \
utils/load_path.cmi \
utils/lazy_backtrack.cmi \
utils/import_info.cmi \
typing/ident.cmi \
utils/consistbl.cmi \
utils/compilation_unit.cmi \
file_formats/cmi_format.cmi \
utils/clflags.cmi \
typing/persistent_env.cmi
typing/persistent_env.cmx : \
utils/warnings.cmx \
typing/subst.cmx \
typing/shape.cmx \
utils/misc.cmx \
parsing/location.cmx \
utils/load_path.cmx \
utils/lazy_backtrack.cmx \
utils/import_info.cmx \
typing/ident.cmx \
utils/consistbl.cmx \
utils/compilation_unit.cmx \
file_formats/cmi_format.cmx \
utils/clflags.cmx \
typing/persistent_env.cmi
typing/persistent_env.cmi : \
typing/subst.cmi \
typing/shape.cmi \
utils/misc.cmi \
parsing/location.cmi \
utils/load_path.cmi \
utils/lazy_backtrack.cmi \
utils/import_info.cmi \
typing/ident.cmi \
utils/consistbl.cmi \
utils/compilation_unit.cmi \
file_formats/cmi_format.cmi
Expand Down
2 changes: 0 additions & 2 deletions testsuite/tests/templates/basic/bad_param_impl.ml
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
ocamlc_byte_exit_status = "2";
compiler_output = "bad_param_impl.output";
ocamlc.byte;
reason = "error broken, will be fixed by #1764";
skip;
compiler_reference = "bad_param_impl.reference";
check-ocamlc.byte-output;
*)
2 changes: 0 additions & 2 deletions testsuite/tests/templates/basic/test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
compiler_output = "bad_ref_direct.output";
ocamlc_byte_exit_status = "2";
ocamlc.byte;
reason = "correct error message not yet implemented";
skip;
compiler_reference = "bad_ref_direct.reference";
check-ocamlc.byte-output;
*)
33 changes: 9 additions & 24 deletions typing/env.ml
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ let map_summary f = function
| Env_value_unbound (s, u, r) -> Env_value_unbound (f s, u, r)
| Env_module_unbound (s, u, r) -> Env_module_unbound (f s, u, r)

type address =
type address = Persistent_env.address =
| Aunit of Compilation_unit.t
| Alocal of Ident.t
| Adot of address * int
Expand Down Expand Up @@ -953,34 +953,24 @@ let components_of_module ~alerts ~uid env ps path addr mty shape =
}
}

let read_sign_of_cmi { Persistent_env.Persistent_signature.cmi; _ } =
let name =
match cmi.cmi_kind with
| Normal { cmi_impl } -> cmi_impl
| Parameter -> Misc.fatal_error "Unsupported import of parameter module"
in
let sign = cmi.cmi_sign in
let flags = cmi.cmi_flags in
let id = Ident.create_persistent (Compilation_unit.name_as_string name) in
let read_sign_of_cmi sign name uid ~shape ~address:addr ~flags =
let id = Ident.create_persistent (Compilation_unit.Name.to_string name) in
let path = Pident id in
let alerts =
List.fold_left (fun acc -> function Alerts s -> s | _ -> acc)
Misc.Stdlib.String.Map.empty
flags
in
let sign = Subst.Lazy.signature Make_local Subst.identity sign in
let md =
{ Subst.Lazy.md_type = Mty_signature sign;
md_loc = Location.none;
md_attributes = [];
md_uid = Uid.of_compilation_unit_id name;
md_uid = uid;
}
in
let mda_address = Lazy_backtrack.create_forced (Aunit name) in
let mda_address = Lazy_backtrack.create_forced addr in
let mda_declaration = md in
let mda_shape =
Shape.for_persistent_unit (name |> Compilation_unit.full_path_as_string)
in
let mda_shape = shape in
let mda_components =
let mty = md.md_type in
components_of_module ~alerts ~uid:md.md_uid
Expand Down Expand Up @@ -1016,7 +1006,7 @@ let check_pers_mod ~loc name =
Persistent_env.check !persistent_env read_sign_of_cmi ~loc name

let crc_of_unit name =
Persistent_env.crc_of_unit !persistent_env read_sign_of_cmi name
Persistent_env.crc_of_unit !persistent_env name

let is_imported_opaque modname =
Persistent_env.is_imported_opaque !persistent_env modname
Expand Down Expand Up @@ -2646,13 +2636,8 @@ let open_signature

(* Read a signature from a file *)
let read_signature modname filename ~add_binding =
let mda =
read_pers_mod modname filename ~add_binding
in
let md = Subst.Lazy.force_module_decl mda.mda_declaration in
match md.md_type with
| Mty_signature sg -> sg
| Mty_ident _ | Mty_functor _ | Mty_alias _ | Mty_strengthen _ -> assert false
let mty = read_pers_mod modname filename ~add_binding in
Subst.Lazy.force_signature mty

let is_identchar_latin1 = function
| 'A'..'Z' | 'a'..'z' | '_' | '\192'..'\214' | '\216'..'\246'
Expand Down
Loading

0 comments on commit ca99fbd

Please sign in to comment.