Skip to content

Conversation

@jrosain
Copy link
Contributor

@jrosain jrosain commented Oct 14, 2025

This PR adds the parsing of elimination constraints as defined in the RFC rocq-prover/rfcs#111.

In particular, it adds the possibility to have sort-level constraints in definitions:

Definition foo@{s s'; | s -> s'} ...

and for global sorts.

Sort s s'.
Constraint s -> s'.
  • Added / updated test-suite.
  • Added / updated documentation.
    • Updated documented syntax by running make doc_gram_rsts.

Overlays (to be merged in sync with the upstream PR)

@jrosain jrosain requested review from a team as code owners October 14, 2025 14:22
@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 Oct 14, 2025
@SkySkimmer
Copy link
Contributor

@coqbot run full ci

@coqbot-app coqbot-app bot removed 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 Oct 14, 2025
@jrosain jrosain added the request: full CI Use this label when you want your next push to trigger a full CI. label Oct 14, 2025
@jrosain jrosain force-pushed the elimination-constraints branch from ef9e78a to 69e04cb Compare October 14, 2025 15:37
@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 Oct 14, 2025
@jrosain jrosain added the request: full CI Use this label when you want your next push to trigger a full CI. label Oct 14, 2025
@jrosain jrosain force-pushed the elimination-constraints branch from 69e04cb to 419734b Compare October 14, 2025 15:45
@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 Oct 14, 2025
@SkySkimmer
Copy link
Contributor

something strange is going on in stdlib_test, the message from https://github.com/rocq-prover/stdlib/blob/e343c58314f5a902e08ec6a211f4e1b5812a8c89/test-suite/output/Fixpoint.v#L123 changed to say to produce a value in sort "Prop" even though the fixpoint is -> bool

jrosain added a commit to jrosain/rocq that referenced this pull request Nov 26, 2025
@jrosain jrosain force-pushed the elimination-constraints branch from 770ece2 to 12d1e20 Compare November 26, 2025 15:00
@coqbot-app coqbot-app bot added needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. and removed 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 Nov 26, 2025
@SkySkimmer
Copy link
Contributor

@coqbot run full ci

@coqbot-app coqbot-app bot removed 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 Nov 27, 2025
jrosain added a commit to jrosain/rocq-lsp that referenced this pull request Nov 28, 2025
@jrosain jrosain added the request: full CI Use this label when you want your next push to trigger a full CI. label Nov 28, 2025
jrosain added a commit to jrosain/rocq that referenced this pull request Nov 28, 2025
@jrosain jrosain force-pushed the elimination-constraints branch from 2e9e71c to 27104aa Compare November 28, 2025 09:00
@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 Nov 28, 2025
@jrosain jrosain added the request: full CI Use this label when you want your next push to trigger a full CI. label Nov 28, 2025
jrosain added a commit to jrosain/rocq-lsp that referenced this pull request Nov 28, 2025
jrosain added a commit to jrosain/rocq that referenced this pull request Nov 28, 2025
@jrosain jrosain force-pushed the elimination-constraints branch from 27104aa to 877688b Compare November 28, 2025 09:44
@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 Nov 28, 2025
Copy link
Member

@mattam82 mattam82 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only have minor comments, I think we will need to play with this code and have a few iterations anyway to ensure its correctness. Just one thing, we don't touch the "broken" transitivity closure issue found by @tabareau here, right? If not I think this can be merged.

| Irrelevant -> Sorts.sprop
| Relevant when Universe.is_type0 u -> Sorts.set
| Relevant -> Sorts.make Sorts.Quality.qtype u
| Relevant -> Sorts.prop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment above the code becomes misleading if you don't change it. I think the solution is fine, but the comment should reflect it.

Polymorphic Definition sort'@{s | u |} := Type@{s|u}.
To help the parser, both `|` in the :n:`@univ_decl` are required.
To help the parser, both `|` in the :n:`@sort_poly_decl` are required.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this outdated? We have @{s;u} already in this branch no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a note about the old syntax (which is still parsed)

@TDiazT
Copy link

TDiazT commented Nov 28, 2025

Just one thing, we don't touch the "broken" transitivity closure issue found by @tabareau here, right?

I think it is being handled by the changes in RigidPaths, and checked in the test file test-suite/success/sort_poly_elim_rigid_paths.v.

@tabareau
Copy link
Contributor

Just one thing, we don't touch the "broken" transitivity closure issue found by @tabareau here, right?

I think it is being handled by the changes in RigidPaths, and checked in the test file test-suite/success/sort_poly_elim_rigid_paths.v.

Yes, it has been fixed by Commit e28c9b8

@mattam82
Copy link
Member

Great!

@jrosain jrosain added the request: full CI Use this label when you want your next push to trigger a full CI. label Nov 28, 2025
@jrosain jrosain force-pushed the elimination-constraints branch from 877688b to dcbdcb9 Compare November 28, 2025 17:31
@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 Nov 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs: fixing The proposed code change is broken.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants