Use Global_module.Name.t to stand for a global module identifier
#1872
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This allows compiling an .mli as a _parameter module_ rather than a normal compilation unit. A parameter module defines a module _type_ rather than a module, so it cannot be referred to directly from another module. A forthcoming PR will add the `-parameter P` option which adds the parameter module `P` as a parameter to the current module, which then allows references to `P` in the module. Further PRs will deal with how to actually use a module that takes parameters. For the moment, `-parameter` is unimplemented, so any reference to a parameter module is an error.
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`).
lukemaurer
commented
Jul 9, 2024
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
changed the title
Global_module.Name.t to stand for a module identifierGlobal_module.Name.t to stand for a global module identifier
lukemaurer
added a commit
that referenced
this pull request
Jul 18, 2024
riaqn
reviewed
Aug 21, 2024
riaqn
reviewed
Aug 21, 2024
riaqn
reviewed
Aug 21, 2024
riaqn
reviewed
Aug 22, 2024
riaqn
reviewed
Sep 26, 2024
riaqn
reviewed
Sep 26, 2024
The failure was in a backtrace with line numbers in the compiler. Fairly certain a better fix would be `git rm`.
riaqn
approved these changes
Sep 26, 2024
lukemaurer
added a commit
to lukemaurer/flambda-backend
that referenced
this pull request
Oct 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #1846.
Currently the
Global_module.Name.ttype never actually carries any arguments, but this PR puts the type everywhere it will need to go in order to make that happen.