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

feat: FunInd: more equalities in context, more careful cleanup #5364

Merged
merged 8 commits into from
Sep 16, 2024

Conversation

nomeata
Copy link
Contributor

@nomeata nomeata commented Sep 16, 2024

A round of clean-up for the context of the functional induction principle cases.

  • Already previously, with match e with | p => …, functional induction would ensure that h : e = p is in scope, but it wouldn’t work in dependent cases. Now it introduces heterogeneous equality where needed (fixes FunInd chokes on pattern match with dependent targets #4146)
  • These equalities are now added always (previously we omitted them when the discriminant was a variable that occurred in the goal, on the grounds that the goal gets refined through the match, but it’s more consistent to introduce the equality in any case)
  • We no longer use MVarId.cleanup to clean up the goal; it was sometimes too aggressive (fixes incorrect functional induction principle #5347)
  • Instead, we clean up more carefully and with a custom strategy:
    • First, we substitute all variables without a user-accessible name, if we can.
    • Then, we substitute all variable, if we can, outside in.
    • As we do that, we look for HEqs that we can turn into Eqs to substitute some more
    • We substitute unused lets.

Breaking change: In some cases leads to a different functional induction principle (different names and order of assumptions, for example).

this fixes #4146

The resulting functional induction principles generated when there is a
`match` in the function definition are far from pretty and contain
suprious copies of local definitions at their non-refined or refined
type. I expect that sometimes these may be useful, so I am hesitant to
remove them too aggressively, so for now it’s better to have too many
assumptions than too few, or even a failure (as in #4146).
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Sep 16, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Sep 16, 2024

Mathlib CI status (docs):

  • ❗ Mathlib CI can not be attempted yet, as the nightly-testing-2024-09-16 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-mathlib, Mathlib CI should run now. (2024-09-16 10:33:27)
  • ✅ Mathlib branch lean-pr-testing-5364 has successfully built against this PR. (2024-09-16 11:43:26) View Log

@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc September 16, 2024 10:42 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 16, 2024
@leanprover-community-bot leanprover-community-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Sep 16, 2024
@nomeata nomeata added this pull request to the merge queue Sep 16, 2024
Merged via the queue into master with commit 445c8f2 Sep 16, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

incorrect functional induction principle FunInd chokes on pattern match with dependent targets
3 participants