ocaml / odoc

Documentation compiler for OCaml and Reason
Other
321 stars 88 forks source link

Some link broken when loading modules from files and not from nested modules #849

Closed nrolland closed 1 year ago

nrolland commented 2 years ago

I noticed a strange difference in behaviors when documented modules are loaded from files, with some href links being broken, compared to having the documented modules taken from nested modules, where the links between referenced modules are preserved.

I made a minimal reproducible example here

Ps : this is a really nice project. thank you for making it

jonludlam commented 2 years ago

I think you've hit an issue that's related to the way dune implements namespacing. It's a bit of a tricky business, which I've tried to document briefly here.

In the particular repro in your repository, you've gone with the default of having dune wrap your library - and in particular you've hand-written the top-level module Odoc_pb.ml that serves as the one module intended to be exposed by your library. As explained in the link above, the other modules in the library are built with mangled names (Odoc_pb__Odoc and Odoc_pb__Odoc_intf), and in order to preserve the short names for these modules, dune first creates and compiles the namespace module odoc_pb__.ml-gen in the _build/default directory. The contents of this file is as follows:

(** @canonical Odoc_pb.Odoc *)
module Odoc = Odoc_pb__Odoc

(** @canonical Odoc_pb.Odoc_intf *)
module Odoc_intf = Odoc_pb__Odoc_intf

This file is compiled first (with -no-alias-deps so we're not required to have compiled Odoc_pb__Odoc or Odoc_pb__Odoc_intf before this one). We then compile the modules Odoc.ml and Odoc_intf.ml with the option -open Odoc_pb__, which brings the short names for your modules into scope, and so your modules can continue to pretend that the modules have been compiled without the name mangling.

The problem here is that dune and odoc need to cooperate in order to maintain the fiction that the namespaced modules are the only ones that exist - so dune annotates the aliases to the modules with the mangled names with the canonical tags you can see above. This is an instruction to odoc that whenever it sees Odoc_pb__Odoc, it should actually replace that with a reference to Odoc_pb.Odoc. When dune is responsible for the toplevel module - ie., nobody has written the x.ml file for a library x, this will all work flawlessly. When the library author has written their own toplevel module (odoc_pb.ml in your case), dune has no choice but to assume the modules have been exposed there, but nothing can ensure this has been done. Odoc will check that that path exists before doing the replacement, and if it doesn't exist it doesn't do it. Since odoc goes to some lengths to pretend these modules with double underscores don't exist, it ends up hiding anything that refers to a module with a double underscore. This is a reasonable stance, since as far as odoc knows, those modules have been intentionally left out of the 'public' interface.

The usual fix is to make sure that everything that is meant to be exposed is available through the top-level module in some way, and that canonical paths are set up to ensure odoc knows how to rewrite references to hidden items.

In your case, the simplest way to do this is to put in the aliases that dune was expecting to be present - ie add these lines to odoc_pb.ml:

module Odoc = Odoc
module Odoc_intf = Odoc_intf

Of course, the reason that the inlined modules don't suffer from this is because none of this name mangling happens!

jonludlam commented 2 years ago

Odoc can detect when these canonical paths don't exist, and we could probably emit some warnings that point out exactly what's going wrong here. This might not be easy as in many cases the aliases have not been put in intentionally - e.g. in base, the module Applicative_intf is not exposed, but this intentional as everything ought to be available through the module Applicative.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. The main purpose of this is to keep the issue tracker focused to what is actively being worked on, so that the amount and variety of open yet inactive issues does not overwhelm contributors.

An issue closed as stale is not rejected — further discussion is welcome in its closed state, and it can be resurrected at any time. odoc maintainers regularly check issues that were closed as stale in the past, to see if the time is right to reopen and work on them again. PRs addressing issues closed as stale are as welcome as PRs for open issues. They will be given the same review attention, and any other help.