Skip to content

Syntactic support for instance names as identifiers #1873

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

Merged
merged 251 commits into from
Oct 1, 2024

Conversation

lukemaurer
Copy link
Contributor

@lukemaurer lukemaurer commented Sep 27, 2023

Depends on #1872.

Currently the only support is as a handwritten extension like

Module(Name1)(Value1)(Name2)(Value2) [@jane.non_erasable.instances]

which isn't great, but initially we plan to support creating and referring to instances only using Dune, so this syntax will only see the light of day in source files generated by Dune and not meant for human consumption.

To support parameterised libraries, it will help to sharpen the distinction
between `Compilation_unit.Name.t` and `Compilation_unit.t`. In particular, a
`CU.Name.t` will mean the name of a .cmi and a `CU.t` will mean the name of a
.cmo/x.

Accordingly, the `cmi_name` field in `Cmi_format` changes from `CU.t` to
`CU.Name.t`. There is never a `CU.t` for a parameter module since it has no
.cmo/x, so for these, the `CU.t` is removed from the `Cmi_format` altogether. A
non-parameter .cmi still needs it, however, since we need to store the pack
prefix for the implementation .cmo/x. (We don't support pack prefixes for
parameter modules.) Accordingly, the `cmi_kind` field now has two variants:

```
type kind =
  | Normal of { cmi_impl : CU.t }
  | Parameter
```

As it happens, all this forces through a related change in `Import_info.t` so
that we can store import info for a parameter module, which has a CRC but no
compilation unit. Since the format of import info for interfaces and
implementations is diverging, a split API is introduced to `Import_info`. (This
is the same API that ocaml-flambda#1746 adds, but here it's optional and only used in the few
places that need it.)
With the advent of parameters, we have reasons to refer to global module names
besides those names being runtime global variables, or addressable values at
all. In addition, we're going to want to load a .cmi but then parameterise it
in different ways to get different signatures, each of which will be bound to
a separate value.

In the end, then, instead of just `pers_struct`, we'll want three different
record types, each with an associated cache:

* `import`, corresponding directly to a .cmi file, keyed by
  `Compilation_unit.Name.t`
* `pers_name`, corresponding to an `import` with parameters applied to it, keyed
  by a `Compilation_unit.Name.t` and some parameters
* `pers_struct`, corresponding to a persistent name that is actually bound in
  the environment, with the same key as `pers_name`

For this PR, I'm leaving out the second cache as nothing about it is relevant
yet, but the split is disruptive enough that even the two are worthwhile. Note
that `pers_struct` is currently still keyed by `Compilation_unit.Name.t` since
we don't yet have the datatypes for parameterised names.

Besides the internal reorganisation of `Persistent_env`, this also changes the
API between it and `Env`, offloading some of the work done by the callback,
`read_sign_of_cmi`. Significantly, `read_sign_of_cmi` is no longer called _at
all_ until something is going to be bound (i.e., become a `pers_struct`). This
keeps the `Env.t` completely free of anything it shouldn't know about.
Currently broken since the `Persistent_env` refactor doesn't have quite the
right abstractions to keep us from calling `Env.read_sign_of_cmi` with
something we're not going to bind in the environment
I've disabled the check on the output, since currently we get the wrong error
message (and the one we get is confusing). This will be much easier to fix when
PR ocaml-flambda#1764 is fixed to avoid unhelpful checks on `.mli` files that are loaded
directly rather than as part of name resolution.
This eliminates a silly bit of back-and-forth, and `Persistent_env` is
perfectly capable of knowing the shape.
This lets us fulfil the intent that we don't call the callback for
things that aren't actually bound in the environment. That way we don't
do awkward things like calculate a binding for something that doesn't
actually have a binding.
Declares that the module being compiled has a parameter, whose type is given by
an .mli. Given `-parameter Param`, we require that:

* An interface `param.mli` is in the path and was compiled with `-as-parameter`
* All subsequent modules that refer to this module will also be compiled with
  `-parameter Param`

Once support for parameterised libraries is complete, it will also be possible
for a reference to pass an argument rather than forwarding `Param` along.
Having it now with a weird type is better than having several weird lines
now (or, that is, soon, when more functions use `print_global_line`).
The library implements a simple little subsitution calculus based on
dependent contextual modal type theory. The library is a functor parameterised
over an identifiable and will calculate what a module's remaining parameters will look
like when that module is referred to with some of its parameters given. Since
parameters can have parameters, the correct behaviour here isn't obvious.
The profiler seems worth at least gesturing toward keeping current. I'm assuming
we're just leaving upstream `objinfo.ml` to rot.
We're not sure how shapes are going to work anyway, so adding
`Shape.of_global_name` seems premature.
lukemaurer added a commit that referenced this pull request Jul 18, 2024
Tip of 'instantiate' (#1905) as of commit 5afb940.

This cherry-pick effectively merges #1872, #1873, #1726, and #1905
onto the 5.1.1minus-18 release (previous cherry-picks already
added #1841 and #1846).
@lukemaurer lukemaurer changed the base branch from lmaurer/global-name-is-module-id to main September 27, 2024 13:41
Copy link
Contributor

@riaqn riaqn left a comment

Choose a reason for hiding this comment

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

LGTM - some small comments.

We will move the syntax to parsetree in a future PR.

@riaqn riaqn merged commit 87e8143 into ocaml-flambda:main Oct 1, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parameterized-libs PRs needed for parameterized libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants