include functor support for OxCaml

[Leonidas-from-XIV]

This PR is a follow up to #1368 and adds support for include functor where it now displays that the items were included via the functor.

[art-w]

Thanks a lot, this looks great! I only have minor feedback but nothing blocking :)

[art-w]

Out of curiosity, can we test the behavior with (** @inline *)?

[jonludlam]

I think there's a chunk of logic missing from this PR. At the moment, all it does AFAICT is get the expansion of the functor and splice it into the expansion of the include. While this works for simple examples, it's not sufficient for more involved cases.

As a simple example, consider this:

module F ( I : sig type t end ) = struct
  type myt = I.t
end

module M = struct
  type t = float
  include functor F
end

Running this through ocamlc -c -i gives what we ought to end up with (roughly):

module F : functor (I : sig type t end) -> sig type myt = I.t end
module M : sig type t = float type myt = float end

If we just simply get the signature of the functor body and splice it in, we end up something more like:

...
module M : sig
  type t = float
  type myt = I.t
end

and obviously that I.t need something done to it. If you run it through odoc as of this branch, it chokes on exactly this:

Exception Failure("Not_found: I/3") handling type_expr: resolved(I/3.t)
backtrace:
Raised at Stdlib.failwith in file "stdlib.ml" (inlined), line 39, characters 17-33
...

I suspect the most straightforward implementation of this would be to do exactly what the docs suggest it does internally. Treat it more like:

module M = struct
  module __DUMMY__ = struct
    type t = float
  end

  include __DUMMY__
  include F(__DUMMY__)
end

We can ensure that the dummy module is hidden, meaning that the include will just inline the contents,
so it should end up rendering in a sensible way. I'm not sure whether it would be best to do this as
a preprocessing step or whether we might be able to rewrite the loaders with an accumulator so we can
do this sort of mangling on loading of the cmt/cmtis.

[Leonidas-from-XIV]

I've now implemented @jonludlam's proposal of compiling down the include functor to basically a BODY module and a functor application. This has some extra steps to deal with odd cases like anonymous functors but seems to work fine. I've also incorporated the example and it compiles now properly.

The way it works is by parsing the include functor as Include.Functor expression (with a path or module type) and then post-processing the structure items if there were include functors and compiling them to Include.Functor with paths (to be able to use `Apply thus avoiding having to infer types) in the shape of <GENERATED_FUNCTOR_MODULE>(GENERATED_WRAPPER_MODULE>). I'm also retaining the original expression, so the include functor <X> will be rendered as include functor <X> instead of include functor APPLY_23(BODY_12).

However there's one outstanding issue: while the items coming from the functor expansions contain the right path (e.g. val functor_output = APPLY_23(BODY_12).functor_output) this is is quite ugly and messy. If I hide the generated modules the cross-reference disappears and just the value remains. This might be fine but also somewhat less informative.

What I could imagine would be best if I could hide the generated modules but also tell the cross-linker that I want the item to be cross-referenced to the ("include") functor, not the generated module. Is there a way to tell odoc to "redirect" paths?

[Leonidas-from-XIV]

Odoc has a mechanism to specify a substitution via @canonical which is represented by the `Canonical constructor, however that can only be constructed with resolved modules, whereas the Typedtree transformation is happening before. Maybe it is possible to resolve these modules early.

[panglesd]

I'll continue the review, but here is already some comments. I think the Make functor should be made a little bit more complex, and in particular use its argument.

module Make (T : sig type t end) : sig type included = T.t end

Now suppose it is used in the following way:

type t
include functor Make

From my understanding of the oxcaml feature, the rendered signature should be:

type t
> include functor Make
  type included = t

(where > is for the opened include accordion 🪗 )
In my testing, that is not the case!

I wonder if it is possible to fix that using the current "sugar". I believe the "desugared" form of the example above (extended with a value and a module) could use aliases, for types, modules and module types, instead of relying on the DUMMY__ include :

type t
val f : t
module X : S
module DUMMY__ : sig
    type nonrec t = t
    val f : t
    module X = X
  end

  module STRUCT__Make : Make
  include module type of STRUCT__Make(DUMMY__)
end

What do you think?

Also, definitely we want those generated modules to be hidden. As @jonludlam mentioned, that would turn the "inlining" invisible if we keep it, and odoc would use links to a visible alias.

Also note that while I'd like to investigate the alternate sugar, the current version is better than no support, and as such (I still need more review of course) I would not block the merge on the problem noted above!

[panglesd]

I had a go at the "alias desugaring" idea: https://github.com/panglesd/odoc/tree/include-functor-alias-desugaring

It looks a reasonable first solution, I think. Maybe implementing the "include functor expansion" mechanism in odoc would be better, but slightly more invasive...

[Leonidas-from-XIV]

I've ended up cherry-picking the commits from @jonludlam's branch, which among an alternative desugaring also solves another real problem that I didn't think of: the BODY module needs to be re-created for every include functor application as one include functor application might depend on items that were generated by a preceding include functor application (e.g. imagine a Identifier include functor that generates (amongst others) a type key and a then a Map include functor which consumes as a type key, the latter needs to see the output of the former).

Also added an additional example of when the body of the module contains an include (a potential issue noticed by @panglesd) and rebased on master (thus running on Oxcaml minus39).

[panglesd]

I have some small comments on the included items in the wrapper module, which needs some testing and (I would bet but not a lot) some implementation.

Otherwise, very good work! Thanks!

[panglesd]

Nothing can refer to them, but without including them, we apply F(BODY_n) without BODY_n having module type what F is expecting.

Maybe that is fine, I don't think odoc will check that as it assumes the compiler has already enforced this, however I think it is worth adding tests (as for instance if later odoc requires BODY_n to have the right signature.
But maybe that is not fine! What happens to the values, if the input module is stored in the expansion?

I think it would be good to add a test with values, ..., classes in the input to the include functor. Moreover, the input functor should alias its input, to test the expansion of the situation described above.

[jonludlam]

Oops, yes, you're right. Nothing stopping the functor from just making an alias of its argument, and then we'd lose all of those other items. My mistake, sorry!

[Leonidas-from-XIV]

I've added a test for class, class types and aliases which exposes this issue:

• The classes and class types refer to the generated BODY__n module. It's somewhat surprising given BODY__n should be hidden, I think there's some is_hidden check missing in some place.
• The Aliased module is constructed properly but indeed missing the class types.