Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for "with module type" constraints #689

Merged
merged 2 commits into from
Jun 28, 2021

Conversation

Octachron
Copy link
Member

This PR adds support for with module type constraint:

module type s = sig module type T module M:T end
module type u = s with module type T := s

and local module type substitution:

module type s:= sig
  type t
  type r
end
module M: s

This change adds a new class of fragments for module types which explains the size of the new code.

There is some cases in xref2/subst.ml that would require more attention during review: there are few assertions about which module type paths can be substituted or not.

One limitation of this PR is that deep destructive substitution is not supported. For instance,

module type s = sig
  module M: sig
     module type r
  end
  module N : M.r
end
with module type M.r := sig end

creates a dead link M.r in the constrained signature. However, this is a global issue with all deep destructive substitutions. It seems better to fix this issue independently.

| Not_replaced p -> p
| Replaced _ ->
(* the left hand side of subst is a named module type inside a module,
it cannot be substituted *)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we're saying here that at the point we're substituting A := B then A has to exist? And this is a constraint that comes from the compiler, so while the types in odoc allow it we should never be in this situation?

@jonludlam
Copy link
Member

This all looks very sensible indeed. Many thanks!

@jonludlam jonludlam merged commit 1a3b77f into ocaml:master Jun 28, 2021
jonludlam added a commit to jonludlam/opam-repository that referenced this pull request Oct 5, 2021
CHANGES:

Breaking changes
- Remove odoc-parser into a separate repository (@jonludlam, ocaml/odoc#700)

Additions
- OCaml 4.13 support (@Octachron, ocaml/odoc#687, ocaml/odoc#689)
- Better errors/warnings (@Julow, ocaml/odoc#692, ocaml/odoc#717, ocaml/odoc#720, ocaml/odoc#732)
- ModuleType 'Alias' support (@jonludlam, ocaml/odoc#703)
- Improved test suite (@lubega-simon, ocaml/odoc#697)
- Improved documentation (@lubega-simon, @jonludlam, ocaml/odoc#702, ocaml/odoc#733)
- Strengthen module types (@jonludlam, ocaml/odoc#731)

Bugs fixed
- `uwt` now can be documented (@jonludlam, ocaml/odoc#708)
- Fix resolution involving deeply nested substitutions (@jonludlam, ocaml/odoc#727)
- Fix off-by-one error in error reporting (@asavahista, ocaml/odoc#736)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants