-
Notifications
You must be signed in to change notification settings - Fork 453
[rocq] Initial support for the Rocq Prover build language #12035
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
base: main
Are you sure you want to change the base?
Conversation
e62bbdb to
2d2d144
Compare
|
I just learned that you are working on this @ejgallego, that's great news! There are several points that are IMO worth discussing before merging this PR:
Also, I'm happy to play a part in this if you want help. |
|
Another candidate is going back to calling |
|
Hey @rlepigre-skylabs-ai !
Indeed! Thanks a lot for your message. We got side-tracked by other stuff, but the idea is to merge this and #11945
Yes, we will forward port this in a separate PR.
IMHO we can just say that both can't work together, there are some folks trying to use this feature, so I'd like to keep it.
This would certainly break a lot of stuff tho, what would we gain?
Happy to work on this but IMHO we can do in a separate PR.
Yes, we will also forward port this.
Absolutely, @Lysxia and I will send a mail soon to coordiante.
This should be easy to do if we want, tho for some users it would mean a large performance regression. But IMHO unrelated to this PR which is to enable Rocq users no to have to depend on the |
I think this was originally due to performance issues in |
Do you have pointer? The original performance problem is that in one rocqdep call per file, rocqdep will still scan the whole user-contrib tree. This has a huge impact in practice, even in fast systems like Linux.
I'd be happy to make this configurable, the original Coq mode used this system so it is easy just to look at the commit and add back that code with a flag. Once we have that we could test for numbers, but my original experiments showed a large overhead of repeated rocqdep calls. |
I can't find anything, but I thought I had seen something in that line. If that wasn't the case, then perhaps |
|
coqdep called by dune (ie with -boot) should not be scanning all user-contrib but only the parts which are mapped to a declared dependency |
|
Are we actually certain that duplicating everything is the right option to support |
I am.
I tried that, but it is actually quite complex, a lot of cases to handle, a lot of different tests, conditional arguments. The breakage is far from trivial.
What is your worry? |
We introduce `(lang rocq)`, a build mode for the Rocq Prover, previously known as "Coq". The build mode is an exact copy of the `(lang coq)` mode, renaming the modules, data, and some global declarations. Also, the default implicit lib is now named `Corelib`. (lang coq): should be considered deprecated / frozen, and eventually removed. There are many cleanups and improvements possible, these will be done in later commits. All of the tests have been ported and manually verified to work (a time consuming process) As of today, all test pass. The test suite has been tested against Rocq 9.1, with the `coq` compatibility package added. Co-authored-by: Li-yao Xia <[email protected]> Signed-off-by: Emilio Jesus Gallego Arias <[email protected]>
For now a copy of Coq's one, with minor updates. Co-authored-by: Li-yao Xia <[email protected]> Signed-off-by: Emilio Jesus Gallego Arias <[email protected]>
Co-authored-by: Li-yao Xia <[email protected]> Signed-off-by: Emilio Jesus Gallego Arias <[email protected]>
Not needed anymore as Rocq dropped support for non-findlib loading. Co-authored-by: Li-yao Xia <[email protected]> Signed-off-by: Emilio Jesus Gallego Arias <[email protected]>
This is smaller than expected, we still need to pass the native options and warnings due to upstream. Co-authored-by: Li-yao Xia <[email protected]> Signed-off-by: Emilio Jesus Gallego Arias <[email protected]>
Beware both `rocq c --config` and `coqc --config` still output `COQ*` variable names. Co-authored-by: Li-yao Xia <[email protected]> Signed-off-by: Emilio Jesus Gallego Arias <[email protected]>
My main worry is asking everyone to move to a new language, for what appears to me to be not a good reason. We'll basically ask people to modify every single of their |
They'll have to do that anyways due to the several improvements in Rocq mode, we would have introduced a new |
e5f5338 to
4ac917c
Compare
Co-authored-by: Li-yao Xia <[email protected]> Signed-off-by: Emilio Jesus Gallego Arias <[email protected]>
We remove the need for installing coqc shims, and adapt the test suite accordingly. There is one mayor caveat and one minor caveat: - the major caveat is that in builds composed with Rocq, dune will only search for `rocq`, but we don't inject a dependency on `rocqworker` which is also needed, but a private binary. This means that users should ensure manually `rocqworker` is built when using composed builds. We hope to fix this soon, in a different PR. - the minor caveat is that with `--display=short` we just now display the `rocq` name, not `rocq compile` or `rocq dep`. This can be also fixed in a different PR. Co-authored-by: Li-yao Xia <[email protected]> Signed-off-by: Emilio Jesus Gallego Arias <[email protected]>
Co-authored-by: Li-yao Xia <[email protected]> Signed-off-by: Emilio Jesus Gallego Arias <[email protected]>
We introduce
(lang rocq), a build mode for the Rocq Prover, previously known as "Coq".The build mode is an exact copy of the
(lang coq)mode, renaming the modules, data, and some global declarations. Also, the default implicit lib is now namedCorelib.(lang coq)should be considered deprecated / frozen, and eventually removed.There are many cleanups and improvements possible, in particular we have done, in separate commits:
All of the tests have been ported and manually verified to work (a time consuming process) As of today, all test pass.
Future work:
(lang coq), as of today the rocqworker is not injected as a dependencyrocqintorocq compilefor--display=shortCloses #11572
Joint work with Li-yao Xia.