Skip to content
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

Apply the early check of the universe declaration and the monomorphic universe fixing to all interactive declarations #18960

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

herbelin
Copy link
Member

@herbelin herbelin commented Apr 19, 2024

Formerly, the early check (at Definition time) of the universe declaration and the monomorphic universe fixing (for asynchronous evaluation) were done only for Theorem and Definition without body (in post_check_evd). We do it also for interactive [Co]Fixpoint and for Program Fixpoint and Program Definition.

To anticipate what is done when proofs are completed, we apply UState.collapse_sort_variables and UState.restrict in some cases (this is needed for the test-suite to pass). But maybe should these be applied in all four cases, together with UState.normalize_variables and UState.minimize, as done at the end of proofs in general???

This gives an alternative solution to #15410 which allows to remove the forbidding for extensible universes introduced in #15424.

This remark at #15410 still hold though: "non extensible universe declarations don't make a lot of sense in universe monomorphic definitions", so maybe it can also be a failure.

  • Added / updated test-suite.
  • TODO: use the same check in all four situations (but which one?)

Depends on #18915 (merged)

@herbelin herbelin added part: program kind: enhancement Enhancement to an existing user-facing feature, tactic, etc. needs: merge of dependency This PR depends on another PR being merged first. request: full CI Use this label when you want your next push to trigger a full CI. part: fixpoints About Fixpoint, fix and mutual statements labels Apr 19, 2024
@herbelin herbelin added this to the 8.20+rc1 milestone Apr 19, 2024
@herbelin herbelin requested review from a team as code owners April 19, 2024 19:57
@coqbot-app coqbot-app bot removed the request: full CI Use this label when you want your next push to trigger a full CI. label Apr 19, 2024
@herbelin herbelin force-pushed the master+early-udecl-check-interactive-proof branch from 8e573bc to 330d02f Compare April 20, 2024 11:04
@coqbot-app coqbot-app bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Apr 20, 2024
@github-actions github-actions bot added the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label Apr 23, 2024
@herbelin herbelin force-pushed the master+early-udecl-check-interactive-proof branch from 330d02f to 4e12d77 Compare April 24, 2024 19:47
@coqbot-app coqbot-app bot removed the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label Apr 24, 2024
@herbelin herbelin force-pushed the master+early-udecl-check-interactive-proof branch from 4e12d77 to 1d98f3a Compare April 24, 2024 20:10
@ejgallego ejgallego added this to TODO in Proof Handling via automation May 6, 2024
@github-actions github-actions bot added the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label May 8, 2024
@herbelin herbelin force-pushed the master+early-udecl-check-interactive-proof branch from 1d98f3a to 63cb197 Compare May 8, 2024 09:12
@coqbot-app coqbot-app bot removed the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label May 8, 2024
@github-actions github-actions bot added the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label May 16, 2024
@herbelin herbelin added request: full CI Use this label when you want your next push to trigger a full CI. and removed needs: merge of dependency This PR depends on another PR being merged first. labels May 17, 2024
@herbelin herbelin force-pushed the master+early-udecl-check-interactive-proof branch from 63cb197 to e9f0806 Compare May 17, 2024 15:59
@coqbot-app coqbot-app bot removed needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. request: full CI Use this label when you want your next push to trigger a full CI. needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. labels May 17, 2024
@herbelin
Copy link
Member Author

In two cases, comDefinition.ml and comProgramFixpoint.ml, I apparently need to do:

let check_univ_decl_early ~poly sigma udecl terms =
let vars = List.fold_left (fun acc b -> Univ.Level.Set.union acc (Vars.universes_of_constr b)) Univ.Level.Set.empty terms in
let uctx = Evd.evar_universe_context sigma in
let uctx = UState.collapse_sort_variables uctx in
let uctx = UState.restrict uctx vars in
UState.check_univ_decl ~poly uctx udecl

But in comFixpoint.ml and vernacentries.ml, collapse_sort_variable is not needed (for the examples of the test-suite to work). Is the need for collapse_sort_variable really expected or is it a revelator of a possible problem elsewhere?

@SkySkimmer
Copy link
Contributor

The examples probably just don't generate free qvars.

@herbelin
Copy link
Member Author

I'm asking because I found sometimes free qsort in the absence of minimization. Now, I also realize that vernacentries.ml and comFixpoint.ml do have a minimization and the others not. So, I have the feeling that what is finally needed is a minimization. I will try, thanks for the help.

@herbelin herbelin force-pushed the master+early-udecl-check-interactive-proof branch from e9f0806 to 1774b5c Compare May 23, 2024 07:21
@coqbot-app coqbot-app bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label May 23, 2024
@herbelin herbelin force-pushed the master+early-udecl-check-interactive-proof branch from 1774b5c to 12fbfa9 Compare May 23, 2024 12:09
@herbelin herbelin requested a review from a team as a code owner May 23, 2024 12:09
@herbelin
Copy link
Member Author

Finally, I used collapse_sort_variable since I saw it is already called by minimization and that, since the result is thrown away after checking, an (a priori) faster function is better.

@github-actions github-actions bot added the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label May 24, 2024
… proofs.

Initially, this was only for "Theorem"-like statements. With the PR,
it is also for interactive Co/Fixpoint and Definition.

We also add in all cases a fix_undefined_universes (it is told to be
needed for asynchronous evaluation).
@herbelin herbelin added the request: full CI Use this label when you want your next push to trigger a full CI. label May 26, 2024
@herbelin herbelin force-pushed the master+early-udecl-check-interactive-proof branch from 12fbfa9 to 959ce8a Compare May 26, 2024 07:49
@coqbot-app coqbot-app bot removed needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. request: full CI Use this label when you want your next push to trigger a full CI. needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. labels May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Enhancement to an existing user-facing feature, tactic, etc. part: fixpoints About Fixpoint, fix and mutual statements part: program
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants