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

Fix support of Program-style pattern-matching for multiple arguments in inductive families #18929

Conversation

herbelin
Copy link
Member

@herbelin herbelin commented Apr 12, 2024

17 years after the first version of Program, this PR debugs the (anticipated) support for multiple patterns involving inductive families. By fixing several de Bruijn indexes bugs, by detecting heteregeneous disequalities, and by addressing a few other typing bugs, it allows for instance to support, without returning strange typing errors, basic examples such as:

Require Import Program Vector.
Program Definition h {A B : Type} {n1 n2 : nat} (v1 : t A n1) (v2 : t A n2) : nat :=
  match v1, v2 with
  | cons _ _ _ _ , _ => 0
  | _, cons _ _ _ _ => 1
  | _, _ => 2
  end.

In particular, it morally fixes #5777 (up to heterogeneous equality discrimination issues still not addressed).

Also fixes #1956.

[More generally, I feel that there is a nice architecture to build by combining Program, Equations, small inversion, which all provide crucial advanced reasoning bricks about inductive types. All of the following are somehow independent parameters that should be freely combinable: Goguen-McBride-McKinna, or Cockx, or small inversion emulation of dependent pattern-matching; adding or not equations and inequations to the typing context; failing or opening obligations or goals on existential variables.]

  • Added / updated test-suite.
  • Added changelog.

Depends on #18921 (merged)

@herbelin herbelin added kind: fix This fixes a bug or incorrect documentation. kind: feature New user-facing feature request or implementation. part: program needs: merge of dependency This PR depends on another PR being merged first. part: pattern-matching compilation request: full CI Use this label when you want your next push to trigger a full CI. labels Apr 12, 2024
@herbelin herbelin added this to the 8.20+rc1 milestone Apr 12, 2024
@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 12, 2024
Copy link
Contributor

coqbot-app bot commented Apr 12, 2024

🔴 CI failure at commit 356f99e without any failure in the test-suite

✔️ Corresponding job for the base commit 9ee4af8 succeeded

❔ Ask me to try to extract a minimal test case that can be added to the test-suite

🏃 @coqbot ci minimize will minimize the following target: ci-metacoq
  • You can also pass me a specific list of targets to minimize as arguments.

@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 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 Apr 18, 2024
@herbelin herbelin force-pushed the master+program-equality-wrapper-multiple-inductive-families branch from 356f99e to 595f14b Compare April 18, 2024 19:01
@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. labels Apr 18, 2024
@herbelin herbelin marked this pull request as ready for review April 19, 2024 14:33
@herbelin herbelin requested a review from a team as a code owner April 19, 2024 14:33
@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+program-equality-wrapper-multiple-inductive-families branch from 595f14b to fb683f9 Compare April 24, 2024 19:51
@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 needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. labels Apr 24, 2024
This was missing when pattern-matching on multiple terms with a
dependent type not in 1st position.
Indeed, we have the invariant "nargeqs + slift = nar".

But the next bugfix commit will change nargeqs so that the invariant
becomes "neqs + nargeqs + nar".
…ith inductive families.

This allows the attached test to compile.
…lities.

We foresee a possible use of JMeq for heterogenous disequality, but
without activating it.
…args of a term to match be evars.

At this stage, the example from issue coq#1956 works.
@herbelin herbelin force-pushed the master+program-equality-wrapper-multiple-inductive-families branch from fb683f9 to 55117d8 Compare May 8, 2024 06:35
@herbelin
Copy link
Member Author

@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 May 13, 2024
@herbelin
Copy link
Member Author

I believe this to be ready.

@herbelin
Copy link
Member Author

Looking for an assignee (it is mostly completing/fixing code which was not yet working; for the details, it might help to look at commits separetedly).

@ppedrot ppedrot self-assigned this Jun 6, 2024
@ppedrot
Copy link
Member

ppedrot commented Jun 6, 2024

@coqbot merge now

@coqbot-app coqbot-app bot merged commit 40e7805 into coq:master Jun 6, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: feature New user-facing feature request or implementation. kind: fix This fixes a bug or incorrect documentation. part: pattern-matching compilation part: program
Projects
None yet
2 participants