Merged
Conversation
None of the changes to code gen can actually be tested without `-instantiate`, which won't be implemented until PR oxcaml#1905. This lets PR oxcaml#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!)
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).
riaqn
reviewed
Oct 28, 2024
riaqn
reviewed
Oct 28, 2024
Parser Change ChecklistThis PR modifies the parser. Please check that the following tests are updated:
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). |
riaqn
reviewed
Oct 31, 2024
riaqn
reviewed
Nov 1, 2024
Contributor
riaqn
left a comment
There was a problem hiding this comment.
More comments; I need to take a closer look at instantiator and translmod and asmpackager.
riaqn
approved these changes
Nov 28, 2024
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
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.
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
-instantiateoption is similar to-packin 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
-ooption 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-instantiateby hand.