Fix bug in arguments to get_unit_info

middle_end/compilenv.ml

@@ -200,11 +200,18 @@ let read_library_info filename =
 
 (* Read and cache info on global identifiers *)
 
-let get_unit_info comp_unit ~cmx_name =
-  if CU.Name.equal cmx_name (CU.name current_unit.ui_unit)
+let get_unit_info comp_unit =
+  (* If this fails, it likely means that someone didn't call
+     [CU.which_cmx_file]. *)
+  assert (CU.can_access_cmx_file comp_unit ~accessed_by:current_unit.ui_unit);
+  (* CR lmaurer: Surely this should just compare [comp_unit] to
+     [current_unit.ui_unit], but doing so seems to break Closure. We should fix
+     that. *)
+  if CU.Name.equal (CU.name comp_unit) (CU.name current_unit.ui_unit)
   then
     Some current_unit
   else begin
+    let cmx_name = CU.name comp_unit in
     try
       CU.Name.Tbl.find global_infos_table cmx_name
     with Not_found ->
@@ -234,13 +241,13 @@ let get_unit_info comp_unit ~cmx_name =
 let which_cmx_file comp_unit =
   CU.which_cmx_file comp_unit ~accessed_by:(CU.get_current_exn ())
 
-let get_unit_export_info comp_unit ~cmx_name =
-  match get_unit_info comp_unit ~cmx_name with
+let get_unit_export_info comp_unit =
+  match get_unit_info comp_unit with
   | None -> None
   | Some ui -> Some ui.ui_export_info
 
 let get_global_info comp_unit =
-  get_unit_info comp_unit ~cmx_name:(which_cmx_file comp_unit)
+  get_unit_info (which_cmx_file comp_unit)
 
 let get_global_export_info id =
   match get_global_info id with
@@ -249,7 +256,7 @@ let get_global_export_info id =
 
 let cache_unit_info ui =
   cache_checks ui.ui_checks;
-  CU.Name.Tbl.add global_infos_table (which_cmx_file ui.ui_unit) (Some ui)
+  CU.Name.Tbl.add global_infos_table (CU.name ui.ui_unit) (Some ui)
 
 (* Return the approximation of a global identifier *)
 
@@ -299,11 +306,12 @@ let flambda2_set_export_info export_info =
 let approx_for_global comp_unit =
   if CU.equal comp_unit CU.predef_exn
   then invalid_arg "approx_for_global with predef_exn compilation unit";
-  let cmx_name = which_cmx_file comp_unit in
+  let accessible_comp_unit = which_cmx_file comp_unit in
+  let cmx_name = CU.name accessible_comp_unit in
   match CU.Name.Tbl.find export_infos_table cmx_name with
   | otherwise -> Some otherwise
   | exception Not_found ->
-    match get_unit_info comp_unit ~cmx_name with
+    match get_unit_info accessible_comp_unit with
     | None -> None
     | Some ui ->
       let exported = get_flambda_export_info ui in

middle_end/compilenv.mli

@@ -63,8 +63,7 @@ val get_global_export_info : Compilation_unit.t -> Cmx_format.export_info option
            .cmx file of the given unit. *)
 
 val get_unit_export_info
-  : Compilation_unit.t -> cmx_name:Compilation_unit.Name.t ->
-      Cmx_format.export_info option
+  : Compilation_unit.t -> Cmx_format.export_info option
 
 val flambda2_set_export_info : Flambda2_cmx.Flambda_cmx_format.t -> unit
         (* Set the export information for the current unit (Flambda 2 only). *)

middle_end/flambda2/cmx/flambda_cmx.ml

@@ -19,25 +19,23 @@ module T = Flambda2_types
 module TE = Flambda2_types.Typing_env
 
 type loader =
-  { get_module_info :
-      Compilation_unit.t ->
-      cmx_name:Compilation_unit.Name.t ->
-      Flambda_cmx_format.t option;
+  { get_module_info : Compilation_unit.t -> Flambda_cmx_format.t option;
     mutable imported_names : Name.Set.t;
     mutable imported_code : Exported_code.t;
     mutable imported_units :
       TE.Serializable.t option Compilation_unit.Name.Map.t
   }
 
 let load_cmx_file_contents loader comp_unit =
-  let cmx_file =
+  let accessible_comp_unit =
     Compilation_unit.which_cmx_file comp_unit
       ~accessed_by:(Compilation_unit.get_current_exn ())
   in
+  let cmx_file = Compilation_unit.name accessible_comp_unit in
   match Compilation_unit.Name.Map.find cmx_file loader.imported_units with
   | typing_env_or_none -> typing_env_or_none
   | exception Not_found -> (
-    match loader.get_module_info comp_unit ~cmx_name:cmx_file with
+    match loader.get_module_info accessible_comp_unit with
     | None ->
       (* To make things easier to think about, we never retry after a .cmx load
          fails. *)

middle_end/flambda2/cmx/flambda_cmx.mli

@@ -20,11 +20,7 @@
 type loader
 
 val create_loader :
-  get_module_info:
-    (Compilation_unit.t ->
-    cmx_name:Compilation_unit.Name.t ->
-    Flambda_cmx_format.t option) ->
-  loader
+  get_module_info:(Compilation_unit.t -> Flambda_cmx_format.t option) -> loader
 
 val get_imported_names : loader -> unit -> Name.Set.t
 

middle_end/flambda2/flambda2.ml

@@ -17,7 +17,8 @@
 (* Unlike most of the rest of Flambda 2, this file depends on ocamloptcomp,
    meaning it can call [Compilenv]. *)
 
-let get_module_info comp_unit ~cmx_name =
+let get_module_info comp_unit =
+  let cmx_name = Compilation_unit.name comp_unit in
   (* Typing information for predefined exceptions should be populated directly
      by the callee. *)
   if Compilation_unit.Name.equal cmx_name Compilation_unit.Name.predef_exn
@@ -30,7 +31,7 @@ let get_module_info comp_unit ~cmx_name =
        |> Compilation_unit.name)
   then None
   else
-    match Compilenv.get_unit_export_info comp_unit ~cmx_name with
+    match Compilenv.get_unit_export_info comp_unit with
     | None | Some (Flambda2 None) -> None
     | Some (Flambda2 (Some info)) -> Some info
     | Some (Clambda _) ->

middle_end/flambda2/flambda2.mli

@@ -28,6 +28,4 @@ val lambda_to_cmm :
   Cmm.phrase list
 
 val get_module_info :
-  Compilation_unit.t ->
-  cmx_name:Compilation_unit.Name.t ->
-  Flambda2_cmx.Flambda_cmx_format.t option
+  Compilation_unit.t -> Flambda2_cmx.Flambda_cmx_format.t option

ocaml/middle_end/compilenv.ml

@@ -137,11 +137,18 @@ let read_library_info filename =
 
 (* Read and cache info on global identifiers *)
 
-let get_unit_info comp_unit ~cmx_name =
-  if CU.Name.equal cmx_name (CU.name current_unit.ui_unit)
+let get_unit_info comp_unit =
+  (* If this fails, it likely means that someone didn't call
+     [CU.which_cmx_file]. *)
+  assert (CU.can_access_cmx_file comp_unit ~accessed_by:current_unit.ui_unit);
+  (* CR lmaurer: Surely this should just compare [comp_unit] to
+     [current_unit.ui_unit], but doing so seems to break Closure. We should fix
+     that. *)
+  if CU.Name.equal (CU.name comp_unit) (CU.name current_unit.ui_unit)
   then
     Some current_unit
   else begin
+    let cmx_name = CU.name comp_unit in
     try
       CU.Name.Tbl.find global_infos_table cmx_name
     with Not_found ->
@@ -172,10 +179,10 @@ let which_cmx_file desired_comp_unit =
   CU.which_cmx_file desired_comp_unit ~accessed_by:(CU.get_current_exn ())
 
 let get_global_info global_ident =
-  get_unit_info global_ident ~cmx_name:(which_cmx_file global_ident)
+  get_unit_info (which_cmx_file global_ident)
 
 let cache_unit_info ui =
-  CU.Name.Tbl.add global_infos_table (which_cmx_file ui.ui_unit) (Some ui)
+  CU.Name.Tbl.add global_infos_table (CU.name ui.ui_unit) (Some ui)
 
 (* Return the approximation of a global identifier *)
 
@@ -221,11 +228,12 @@ let set_export_info export_info =
 let approx_for_global comp_unit =
   if CU.equal comp_unit CU.predef_exn
   then invalid_arg "approx_for_global with predef_exn compilation unit";
-  let cmx_name = which_cmx_file comp_unit in
+  let accessible_comp_unit = which_cmx_file comp_unit in
+  let cmx_name = CU.name accessible_comp_unit in
   match CU.Name.Tbl.find export_infos_table cmx_name with
   | otherwise -> Some otherwise
   | exception Not_found ->
-    match get_unit_info comp_unit ~cmx_name with
+    match get_unit_info accessible_comp_unit with
     | None -> None
     | Some ui ->
       let exported = get_flambda_export_info ui in

ocaml/utils/compilation_unit.ml

@@ -303,38 +303,41 @@ let can_access_by_name t ~accessed_by:me =
   in
   t's_prefix_is_my_ancestor && t_is_not_my_strict_ancestor
 
-let which_cmx_file desired_comp_unit ~accessed_by : Name.t =
+let can_access_cmx_file = can_access_by_name
+
+let which_cmx_file desired_comp_unit ~accessed_by : t =
   let desired_prefix = for_pack_prefix desired_comp_unit in
   if Prefix.is_empty desired_prefix
   then
     (* If the unit we're looking for is not in a pack, then the correct .cmx
        file is the one with the same name as the unit, irrespective of any
        current pack. *)
-    name desired_comp_unit
+    desired_comp_unit
   else
     (* This lines up the full paths as described above. *)
-    let rec match_components ~current ~desired =
+    let rec match_components ~current ~desired ~acc_rev =
       match current, desired with
       | current_name :: current, desired_name :: desired ->
         if Name.equal current_name desired_name
         then
           (* The full paths are equal up to the current point; keep going. *)
-          match_components ~current ~desired
+          let acc_rev = current_name :: acc_rev in
+          match_components ~current ~desired ~acc_rev
         else
           (* The paths have diverged. The next component of the desired path is
              the .cmx file to load. *)
-          desired_name
+          acc_rev, desired_name
       | [], desired_name :: _desired ->
         (* The whole of the current unit's full path (including the name of the
            unit itself) is now known to be a prefix of the desired unit's pack
            *prefix*. This means we must be making a pack. The .cmx file to load
            is named after the next component of the desired unit's path (which
            may in turn be a pack). *)
-        desired_name
+        acc_rev, desired_name
       | [], [] ->
         (* The paths were equal, so the desired compilation unit is just the
            current one. *)
-        name desired_comp_unit
+        acc_rev, name desired_comp_unit
       | _ :: _, [] ->
         (* The current path is longer than the desired unit's path, which means
            we're attempting to go back up the pack hierarchy. This is an
@@ -344,8 +347,14 @@ let which_cmx_file desired_comp_unit ~accessed_by : Name.t =
            unit@ %a"
           print desired_comp_unit print accessed_by
     in
-    match_components ~current:(full_path accessed_by)
-      ~desired:(full_path desired_comp_unit)
+    let prefix_rev, name =
+      match_components ~current:(full_path accessed_by)
+        ~desired:(full_path desired_comp_unit)
+        ~acc_rev:[]
+    in
+    (* CR lmaurer: It's silly to be writing `ListLabels` out everywhere,
+       especially here. *)
+    create (ListLabels.rev prefix_rev |> Prefix.of_list) name
 
 let print_name ppf t = Format.fprintf ppf "%a" Name.print (name t)
 

ocaml/utils/compilation_unit.mli

@@ -139,14 +139,22 @@ val is_parent : t -> child:t -> bool
     * [A.Q] _cannot_ access [F.G] (by criterion 1) or [A] (by criterion 2). *)
 val can_access_by_name : t -> accessed_by:t -> bool
 
+(** A clearer name for [can_access_by_name] when the .cmx file is what's of
+    interest. *)
+val can_access_cmx_file : t -> accessed_by:t -> bool
+
+(*_ CR-someday lmaurer: Arguably [which_cmx_file] should return a different
+  type, since "compilation unit for which we can load the .cmx" is an important
+  constraint. *)
+
 (** Determine which .cmx file to load for a given compilation unit.
     This is tricky in the case of packs.  It can be done by lining up the
     desired compilation unit's full path (i.e. pack prefix then unit name)
     against the accessing unit's full path and observing when/if they
     diverge.
 
     This is only used for native code compilation. *)
-val which_cmx_file : t -> accessed_by:t -> Name.t
+val which_cmx_file : t -> accessed_by:t -> t
 
 (** A distinguished compilation unit for initialisation of mutable state. *)
 val dummy : t