Typing for parameterised libraries #1726
Conversation
mshinwell
added
typing
parameterized-libs
|
This is a big PR! What’s the review plan here? Maybe this question is premature (feel free to tell me to look away), but I’m curious if any other type-systems person has this on their radar… |
Step one is certainly for me to split it up. (I just did a bit of that with #1746, which turns out to be largely orthogonal.) I had things nicely split into a stack of branches, but everything's so interrelated that it got impractical to keep them split up, but now that things are actually working I feel a bit more confident drawing lines around things. (That said, it's annoyingly possible that instantiation will require yet more changes to this code. But at least I can write meaningful tests against this PR, which is more than I can say for, say, just the syntax extension.) |
lukemaurer
force-pushed
the
instance-typing
branch
from
d31f18f to
2eb7bbf
Compare
Displaying the correct error message will be easier again after ocaml-flambda#1764
This reverts commit 6f86fb9.
This both makes `test.ml` _far_ easier to maintain and will also make testing native compilation easy without copying everything. Pulled from the 'instantiate' branch, commit fab77b5.
Pulled from the 'instantiate' branch, commit 454de0d.
Pulled from 'instantiate' branch, commit a9f9196.
Punted on conflicts in `test.ml` since we're switching to generating it.
ocaml/testsuite/tests/templates/basic/bad_instance_arg_name_not_found.reference
Outdated
Show resolved
Hide resolved
ocaml/testsuite/tests/templates/basic/bad_instance_arg_value_not_arg.reference
Outdated
Show resolved
Hide resolved
ocaml/testsuite/tests/templates/basic/bad_instance_arg_value_not_found.reference
Outdated
Show resolved
Hide resolved
ocaml/testsuite/tests/templates/basic/bad_instance_arg_value_wrong_type.reference
Outdated
Show resolved
Hide resolved
ocaml/testsuite/tests/templates/basic/bad_ref_direct_imported.reference
Outdated
Show resolved
Hide resolved
Previously this was pretty rare (only one existing test needed to be promoted), with the exception of `Not_found` which has special handling, but parameterised modules add new and interesting ways for the user to mess up, and we want those errors to have location information (which `Persistent_env` functions aren't aware of).
Also exercises a couple of new cases: passing the same name as `-parameter` and `-as-argument-for` (i.e., implementing operators over parameter modules), and having a parameter module just include an `_intf.S`. (The latter isn't technically difficult but I expect it to be an important use case.)
Currently the mode should always be `legacy` but this wasn't being checked. Also quit passing `path` around in a few places where we know full well the path is just a single global identifier.
ocaml/testsuite/tests/templates/basic/bad_instance_arg_value_wrong_type.reference
Outdated
Show resolved
Hide resolved
ocaml/testsuite/tests/templates/basic/bad_instance_arg_value_not_arg.reference
Outdated
Show resolved
Hide resolved
ocaml/testsuite/tests/templates/basic/bad_param_not_param.reference
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I think it's good to go.
A gentle reminder that maybe we want to move some changes from the next PR to the current PR:
#1905 (comment)
#1905 (comment)
I have no strong opinion - maybe it's not worthwhile.
Typechecking of references to parameterised libraries, with or without arguments. Currently such references can't actually be compiled to working code because we don't have the
-instantiatepass, but this contains all the typechecking logic to make sure that, for instance, a moduleAcan't refer to a parameterised moduleBunlessAtakes all the parameters thatBdoes (the subset rule) or the reference includes an argument for each missing parameter.