compilerlibs: avoid unnecessary read to +compiler-libs/topdirs.cmi

[Octachron]

This PR proposes to modifiy the pre-initialization of the toplevel in order to not force clients of compiler-libs.toplevel to read +compiler-libs/topdirs.cmi. (This is the non-hackish alternative to #11318.)

Currently, the ocaml toplevel is initialized in two phases:

• when the Toploop module is linked (by a call to Topeval.init)
• before the main loop is started by Topmain.main

The first pre-initialization is done to ensure that the toplevel is at least partially initialized when building custom toplevel with ocamlmktop. A good illustration of such use case would to build a custom toplevel with a custom int printer

(* printer.ml *)
let pi ppf = Format.fprintf  ppf "%03d" ;;

which is automatically loaded in another module

(* loader.ml *)
Topdirs.dir_install_printer Format.err_formatter (Longident.Ldot(Lident "Printer_bis","pi"))

and the custom toplevel is then build with

ocamlmktop printer.cmo loader.cmo -o mytop

When #11199 moved topdirs.cmi to +compiler-libs, it kept compatibility with this use case by loading +compiler-libs/topdirs.cmi in the pre-initialization phase. However, this means that nearly all programs linked with ocamltoplevel.cma will try to read this file as soon as they load Toploop without any control on this behavior. This unavoidable cmi read created some stability in our testsuite (see #11318) but at a higher-level is seem strange to enforce such read to all users for the sake of an edge case.

This PR proposes thus to:

• remove the read to +compiler-libs/topdirs.cmi during the pre-initialization of the toplevel
• restore compatibility with ocamlmktop by reading +compiler-libs/topdirs.cmi if we are trying to install a printer and if Topdirs is not in the toplevel environment.

A possible alternative would be to go further in the direction of the second commit and makes the directive for installing printer fully responsible of loading topdirs.cmi (with some caching of the type of printers to avoid reading the cmi at every printer installation). The major difference with this PR approach is that the Topdirs module would no longer be always accessible from the toplevel.

[gasche]

Doing something magic on printer installation also sounds like a hack to me.

A naive idea would be as follows:

• have a compiler-libs module toplevel_init.cmo that does nothing else than let () = Topeval.init
• have ocamlmktop automatically link this module before user-provided modules
• don't link this module in compiler-libs, but call Topeval.init from Toploop; this probably requires ensuring that calling Topeval.init a second time is a no-op

I think that we do something similar with std_exit which is implicitly linked after user code.

[Octachron]

The binary ocamlmktop is a fine shell wrapper around ocamlc which is already linking a topstart module at the end of the user modules to start the toplevel. It is perfectly possible to link another module at the start of this list (Ocamlktop_topenv_init?).

[gasche]

The new approach looks nice, thanks! See inline comment.

[gasche]

Out of curiosity: why are topstart and ocamlmktop_init handled so differently in the build system and in the installation layout? I would assume that we could use the same approach for both modules, making the overall system simpler.

[Octachron]

The Topstart module is used by the toplevel itself, it makes sense to distribute as part of the compiler library. Since ocamlmktop is the only user of the new Ocamlmktop_init module, I feel that it is better to distribute it as a part of a small "ocamlmktop runtime library".

[gasche]

You could have a topinit module to complement topstart, and then build ocaml exactly as the result of ocamlmktop called with no user-provided module at all.

[dra27]

I agree with @gasche - given that topdirs.cmi has to be special-cased by OS packagers now (i.e. +compiler-libs/topdirs.cmi must always be installed, even in layouts which install the compiler-libs separately) it doesn't seem too bad to put this module there too?

[Octachron]

I would rather not increase the use of purely effectful module triggered by linking in the toplevel. It seem better to let Topmain responsible for the initialization of the paths and the directives.

The ocamlmktop_init module is a backward-compatibility pre-initialization step that is only required by ocamlmktop and I would rather contain it to ocamlmktop.

[dra27]

I don't think this will affect utop, but have you compiled utop with it?

[Octachron]

I have not compiled utop with this patch, but since utop has already been patched to call Topcommon.load_topdirs_path, it should not be affected in anyway?

[gasche]

My approval still stands. My gut feeling is that the approach could be further simplified by pulling down the thread of the initialization logic (see our discussion inline), but @Octachron has looked more deeply than I have and seems unconvinced -- or maybe he had enough exciting realizations about how the toplevel work and is eager to move to something else for a bit.

In any case, the current PR is an improvement over the ugly hack that is currently in the codebase, so it deserves to be merged in any case.

[gasche]

@Octachron may want to cleanup the commit history.

I think this PR could deserve a Changes entry, it makes an observable change to the installation layout etc.

[Octachron]

Speaking of observable change in the installation layout, the Makefile change is not quite right for bytecode only installation. I will fix that shortly.