OxCaml: support for parameterized libraries by art-w · Pull Request #1464 · ocaml/odoc

art-w · 2026-07-23T11:25:00Z

This PR integrates OxCaml parameterized libs in Odoc, by adding the missing informations (is that library a description of a parameter? is that library parameterized and by what? does that library implement a parameter?) and fixing the broken references to instantiated parameterized libraries. The later is similar to a functor application, with the difference that arguments are nominal instead of positional: The OxCaml compiler internally uses the syntax Lib[Param:Impl], which we replicate (with individual links on Lib, Param and Impl).

See the cram test or dune's documentation on parameterized libs

Fixes #1390

The odoc generation was skipping all libraries with an `implements` field (= assuming virtual modules), which meant that the documentation of OxCaml parameterized libraries was missing important pages (see ocaml/odoc#1464)

art-w · 2026-09-01T11:07:24Z

+
+Everything builds and odoc generates documentation for all the libraries:
+
+  $ dune build @doc-private 2>&1


Using @doc-private because otherwise dune skips the documentation of libraries with an implements (fixed in the next release of dune by ocaml/dune#16200 )

I think this is a good hint, maybe worth writing that rationale in the test.

Also, come to think of it, it might be worth adding a constraint on dune versions in the OPAM file in the case of with-test (and maybe if we can filter on the oxcaml compiler, so we don't bump the minimal dune version for tests on OCaml)

art-w · 2026-09-01T11:07:44Z

Rebased for OxCaml minus39

Leonidas-from-XIV

Looks overall very nice, I only have a few nitpicks.

I am a bit thrown by the spelling of "parameterisation" since I would've used "parametrization". I think it makes sense to adopt the spelling that OxCaml uses, however the docs never seem to mention it so I don't know what their preferred spelling is.

Leonidas-from-XIV · 2026-09-01T14:50:02Z

+
+Everything builds and odoc generates documentation for all the libraries:
+
+  $ dune build @doc-private 2>&1


I think this is a good hint, maybe worth writing that rationale in the test.

Also, come to think of it, it might be worth adding a constraint on dune versions in the OPAM file in the case of with-test (and maybe if we can filter on the oxcaml compiler, so we don't bump the minimal dune version for tests on OCaml)

Leonidas-from-XIV · 2026-09-01T14:51:33Z

+
+Render to markdown for inspection:
+
+  $ for f in $(find _build/default/_doc/_odocls -name '*.odocl' | sort); do


I wonder if there's a way to get this without having to inspect the internals of _build? Just to make sure it's not unnecessarily fragile.

Leonidas-from-XIV · 2026-09-01T14:52:32Z

+The library names below are suffixed with an opaque hash by dune; we normalise
+it away so the output is stable.
+
+  $ md() { cat markdown/$1@*/$2.md | sed 's/@[0-9a-f]*/@HASH/g'; }


Useless use of cat potentially. But I see the improved readability aspect of it.

Leonidas-from-XIV · 2026-09-01T14:58:01Z

+  >   odoc html-generate --indent --sidebar sidebar.odoc-sidebar -o html \
+  >     $(find _build/default/_doc/_odocls -iname "$lib.odocl")
+  > done
+  $ grep -rl 'current_unit' html/ | sed 's/@[0-9a-f]*/@HASH/g' | sort


OpenBSD grep discourages -r and encourages usage of -R, however that has slightly different semantics in GNU grep. Potentially worth switching to -R, however its not super important.

Leonidas-from-XIV · 2026-09-02T09:12:03Z

+
+and instance_prefix env (m : Paths.Path.Module.t) : Paths.Path.Module.t option =
+  let unfolded = unfold_alias env m in
+  if unfolded = m || not (is_instance unfolded) then None


Is = safe to use on Path.Path.Module.t values?

Leonidas-from-XIV · 2026-09-02T09:31:27Z

+  | None -> raise (Cmt_format.Error (Cmt_format.Not_a_typedtree filename))
+#else
+let read_cmt_and_parameterisation filename =
+  (no_parameterisation, Cmt_format.read_cmt filename)


Small nitpick but the order of items in the name of the function and in the tuple are swapped.

Leonidas-from-XIV · 2026-09-02T09:33:58Z

+  in
+  match cmt with
+  | Some cmt -> (parameterisation, cmt)
+  | None -> raise (Cmt_format.Error (Cmt_format.Not_a_typedtree filename))


Maybe have this at the top, since no point of figuring out a parameterisation if cmt is None from the get-go.

Also, why is the error Not_a_typedtree?

Leonidas-from-XIV · 2026-09-02T09:35:59Z

    List.mem id env.shadowed
 module Path = struct

+let module_of_id id = `Root (ModuleName.of_ident id)


Maybe it would be more readable to have this in the #else block, to avoid shadowing it in the OXCAML case.

Leonidas-from-XIV · 2026-09-02T09:37:48Z

+#if defined OXCAML
+  let rec read_global_name (n : Global_module.Name.t) : Paths.Path.Module.t =
+    (* OxCaml parameterized library application "Lib[Param:Impl][P2:I2]" *)
+    let base = `Root (ModuleName.make_std n.head) in


That would be Lib in this example, correct?

Leonidas-from-XIV · 2026-09-02T09:59:49Z

  (Comment.standalone preamble, Comment.standalone first_comment @ items)

-let make_expansion_page ~source_anchor url comments items =
+let make_expansion_page ?(library_parameter = false) ~source_anchor url comments


I wonder whether it wouldn't be better to make this a mandatory named param, to avoid accidentally setting this to false by leaving it out. But maybe there's really just one place where it should be set to true.

jonludlam · 2026-09-03T16:45:11Z

Thanks @art-w ! sorry it's taken me some time to get to this. I think the loader parts look good, and the rendering looks mostly fine (modulo the boolean tacked onto the Page.t).

I'm more hesitant about the resolution parts. I was expecting there to be more changes in tools.ml where we resolve paths, but you're essentially throwing an error there and trying to handle it in link.ml instead. Aside from being in a surprisingly different place, it also means that we don't resolve subpaths, like Lib[Param:Impl].Inner as when we reresolve in link.ml they don't match `ApplyParam. I'd like to see how it looks to handle everything in tools.ml - I've had a very rough attempt here though I'd suggest using that as a suggested direction rather than something to cherry pick.

For the boolean in Page.t, I think it might be better if we get a new kind variant in document/url.ml and then we can just amend make_name_from_path rather than prefixing.

art-w mentioned this pull request Aug 25, 2026

fix(oxcaml): odoc generation for parameter implementations ocaml/dune#16200

Merged

OxCaml: test parameterized libraries

e5305a1

art-w force-pushed the oxcaml-param branch from 0b677ed to 1f339fb Compare September 1, 2026 10:21

art-w commented Sep 1, 2026

View reviewed changes

art-w requested a review from Leonidas-from-XIV September 1, 2026 11:07

OxCaml: support for parameterized libraries

f23b50b

art-w force-pushed the oxcaml-param branch from 1f339fb to f23b50b Compare September 1, 2026 12:57

Leonidas-from-XIV reviewed Sep 2, 2026

View reviewed changes


		Everything builds and odoc generates documentation for all the libraries:

		$ dune build @doc-private 2>&1


		Render to markdown for inspection:

		$ for f in $(find _build/default/_doc/_odocls -name '*.odocl' \| sort); do

Uh oh!

Conversation

art-w commented Jul 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

art-w commented Sep 1, 2026

Uh oh!

Leonidas-from-XIV left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonludlam commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants