Remove legacy combined Import_info.t type and update all uses
#2552
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.)
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.
Displaying the correct error message will be easier again after ocaml-flambda#1764
[Import_info.t] is now split between the intf and impl cases, which will make it easier for a subsequent PR (which will likely supersede ocaml-flambda#1746 and ocaml-flambda#1933) to pull apart the types and force all callers to choose. Also added a [Consistbl]-centric API for each of [Intf] and [Impl] to make it easy again to convert between import info and an entry in a [Consistbl].
This was referenced May 9, 2024
…ort-info-split-v2
This function was left out as an oversight and several compiler-libs clients seem to need it.
Closed
lukemaurer
force-pushed
the
import-info-split-v2
branch
from
07d29e9 to
a3e7f5b
Compare
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.
#1753 introduced a split API to
Import_infothat distinguishes between imports of interfaces (i.e., .cmi imports) and imports of implementations (i.e., .cmx imports), but it retained the old combined API to minimize the changes. This PR does the remaining work of taking away the old type, which forces every occurrence ofImport_infoto pick one or the other. Because very nearly every value ofImport_info.twas statically one flavor of import or the other, almost everything in this PR (outside ofimport_info.ml) just adds either.Intfor.Implto something.Supersedes #1746 and #1933, which did something similar with an older version of the #1753 API.