Skip to content

Add -instantiate #1905

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 465 commits into from
Nov 28, 2024
Merged

Add -instantiate #1905

merged 465 commits into from
Nov 28, 2024

Conversation

lukemaurer
Copy link
Contributor

@lukemaurer lukemaurer commented Oct 5, 2023

This is the last piece of compiler support for parameterised libraries: the ability to take a parameterised module and a set of arguments and produce an instance module. The -instantiate option is similar to -pack in that it specifies a special compilation mode that takes in .cmo or .cmx files and produces a new one from them. It takes the module to instantiate and the modules to provide as instance arguments.

One bit of awkwardness is that the -o option is required, but (up to directory) there's no actual choice: the output module must have the correct form to be usable from other modules so that .cmx lookup can function. This would be somewhat awkward to fix, and it's not worth doing so at the moment since we don't intend for humans to be running -instantiate by hand.

@lukemaurer lukemaurer added the parameterized-libs PRs needed for parameterized libraries label Oct 5, 2023
@lukemaurer lukemaurer changed the base branch from main to lmaurer/instance-typing October 19, 2023 18:01
None of the changes to code gen can actually be tested without
`-instantiate`, which won't be implemented until PR ocaml-flambda#1905. This lets PR ocaml-flambda#2177
just be about typechecking, which we _can_ test immediately (in that we can
check that if you compile with `-as-argument-for` then you have to conform to
the parameter type and we remember that you passed the flag).
For each runtime parameter `P`, this generates a Lambda binding

  let P = raise (Invalid_argument "-parameter not yet implemented")

This, of course, makes the module unusable when linked, but we can still test
that compilation goes through. (Also, linking directly against a parameterised
module _should_ raise an error!)
This effectively reverts the changes from the following commits:

* e8abb3a
* 1cee4f4
* 814a8a7
* 95e7772

Mostly these changes just dropped code gen or added stub code, so we're happy to
revert them. The last one did add some tests, so I'll be un-reverting it.
The test currently fails, as linking shouldn't be allowed against a
parameterised module. (The module will harmlessly load and do nothing, since
it's just a block with a function, but silent failure is bad.)

A partial cherry-pick from commit 95e7772.
Removal of code gen from this branch of parameterised libraries means the list
of runtime parameters is no longer stored in the .cmo (yet).
@lukemaurer lukemaurer changed the base branch from lmaurer/instance-typing to main October 28, 2024 12:02
Copy link

github-actions bot commented Oct 28, 2024

Parser Change Checklist

This PR modifies the parser. Please check that the following tests are updated:

  • parsetree/source_jane_street.ml

This test should have examples of every new bit of syntax you are adding. Feel free to just check the box if your PR does not actually change the syntax (because it is refactoring the parser, say).

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.

More comments.

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.

More comments; I need to take a closer look at instantiator and translmod and asmpackager.

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.

Looked at all code

@lukemaurer lukemaurer merged commit 4598ac5 into ocaml-flambda:main Nov 28, 2024
19 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