I'm not sure about how this will be used, but don't you want to keep more information also for recursive modules, for functor's body, for local modules, etc?

@alainfrisch Yes, we should do all of those indeed. (I will try to add this)

As for how this is used: it's used for prefixing variable and function names so that in the debugger full paths appear correctly. For example, a toplevel value named "bar" in module "Foo" will be available in the debugger as "Foo.bar" (even though Flambda may have lifted the definition to a symbol). Function names are also prefixed.

An extended notion of path is actually used in the debugger to allow the identification of subfunctions, though that isn't relevant to this patch. For example the function "baz" whose static scope is "Foo.bar" will be identified as "Foo.bar.baz". Locations are used instead of names if the function is anonymous.

It isn't clear to me that it is worth spending time to implement this various support on the Closure path, given the efforts that are happening to try to make Flambda fast enough in -Oclassic mode.

The decision to drop the Closure path does not really depend on the amount of efforts put into improving the speed of -Oclassic, but rather on the outcome of these efforts, which -- I guess -- are hard to predict. And even if good compilation-time can be achieved, flambda is still evolving quite fast and has not been fully battle-tested yet, so I would argue to keep Closure for some time anyway.

That said, it seems perfectly fine to declare that the advanced debugging experience will only be available to those deciding to switch to flambda.

@alainfrisch I've addressed your comments together with the addition of support for local and recursive modules. Could you please look this over again? As an example:

module Normal = struct
  type t = int

  let f (t : t) = t

  module Submodule = struct
    let f2 (t : t) = t
  end
end

module Functor (X : sig type t end) = struct
  type z = X.t

  let g (x : z) = x
end

let toplevel_fn () =
  let module M = struct
    let in_local_module x = x + 42
  end in
  M.in_local_module 3

module rec Rec1 : sig
  val in_rec1 : int -> int
end = struct
  let in_rec1 x = x + 1
end and Rec2 : sig
  val in_rec2 : int -> int
end = struct
  let in_rec2 x = x + 2
end

produces:

(let
  (Normal/1204 =
     (under-module-path(Testmod.Normal) testmod.ml(1):0-120
       (let
         (f/1206 = (function t/1207 t/1207)
          Submodule/1208 =
            (under-module-path(Testmod.Normal.Submodule) testmod.ml(6):62-116
              (let (f2/1209 = (function t/1210 t/1210))
                (makeblock 0 f2/1209))))
         (makeblock 0 f/1206 Submodule/1208)))
   Functor/1211 =
     (under-module-path(Testmod.Functor) testmod.ml(11):122-206
       (function X/1213 is_a_functor
         (let (g/1215 = (function x/1216 x/1216)) (makeblock 0 g/1215))))
   toplevel_fn/1217 =
     (function param/1221
       (let
         (M/1220 =
            (in-local-module(M/1220) testmod.ml(18):242-243
              (let (in_local_module/1218 = (function x/1219 (+ x/1219 42)))
                (makeblock 0 in_local_module/1218))))
         (apply (field 0 M/1220) 3)))
   Rec1/1222 =
     (apply (field 0 (global CamlinternalMod!)) [0: "testmod.ml" 25 6]
       [0: [0: 0a]])
   Rec2/1223 =
     (apply (field 0 (global CamlinternalMod!)) [0: "testmod.ml" 29 6]
       [0: [0: 0a]]))
  (seq
    (apply (field 1 (global CamlinternalMod!)) [0: [0: 0a]] Rec1/1222
      (under-module-path(Testmod.Rec1) testmod.ml(23):320-409
        (let (in_rec1/1230 = (function x/1231 (+ x/1231 1)))
          (makeblock 0 in_rec1/1230))))
    (apply (field 1 (global CamlinternalMod!)) [0: [0: 0a]] Rec2/1223
      (under-module-path(Testmod.Rec2) testmod.ml(27):410-492
        (let (in_rec2/1232 = (function x/1233 (+ x/1233 2)))
          (makeblock 0 in_rec2/1232))))
    (makeblock 0 Normal/1204 Functor/1211 toplevel_fn/1217 Rec1/1222
      Rec2/1223)))

It looks like the character positions may be too large for recursive modules, but that can be fixed separately.

The intention is that local modules will have their path completed upon entry to Flambda. That process will take the current module and function path (e.g. Foo.f) and prepend it to the local module path from the event to form the new current module path (e.g. Foo.f.M). This will be dealt with in a subsequent patch.

Could you please look this over again?

I'm really not familiar with debugging events and how they are used, so it would be better if someone more familiar with these topics can have a look.

Being able to give a nice name, including for local modules, is a very good direction, but it would be nice it this could also be used for synthesizing the internal name attached to exception/extension constructors. Do you think your current approach would help for this goal as well? (I'm a bit concerned about the fact that support for local module is related to flambda, for instance.)

It seems safe. I think it can be merged after rebasing.

@alainfrisch I'm not sure about exception/extension constructor names; I will have to leave that for someone else to investigate. We need to make progress with merging these patches for the native debugging work...

I've made a few simplifications (there was some potential confusion between absolute and relative module paths before). We may need to tweak this some more once all of the debugging work has been merged, but I think this is ok for now.