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

Strengthen module types #731

Merged
merged 2 commits into from
Sep 24, 2021
Merged

Conversation

jonludlam
Copy link
Member

I had thought I'd already done this!

@jonludlam jonludlam force-pushed the strengthen-module-types branch from 5944b44 to 4a4be2f Compare September 22, 2021 16:32
@jonludlam
Copy link
Member Author

I think we might still be missing strengthening of class types too

Copy link
Collaborator

@Julow Julow left a comment

Choose a reason for hiding this comment

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

LGTM

| None -> Some (ModuleType.Path { p_path = prefix; p_expansion = None })
| Some _ -> m.expr
in
let expr = Some (ModuleType.Path { p_path = prefix; p_expansion = None }) in
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we keep m.expr into p_expansion ? What if m.expr is something complicated like With or TypeOf ?

Copy link
Member Author

@jonludlam jonludlam Sep 23, 2021

Choose a reason for hiding this comment

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

Even if it was something complicated before, it's simple now, and calculating the signature or expansion just looks at p_path and gets it's expansion - here and here

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alright, we don't need to keep the expansion around because it's getting recomputed anyway.

When strengthening a module, we now 'strengthen' module types too,
so for all module types declared in `X`, if we're strenthening with
`Y`, we replace e.g. for module type `A` in `X`, `X.A = sig ... end`
with `X.A = Y.A`.

These 'module type aliases' are then not expanded, resulting in a
reduction in number of output files.
@jonludlam jonludlam force-pushed the strengthen-module-types branch from 4a4be2f to 0b2facd Compare September 24, 2021 20:28
@jonludlam jonludlam merged commit af0a35c into ocaml:master Sep 24, 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