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: same equational lemmas for recursive and non-recursive functions #5129

Merged
merged 54 commits into from
Aug 25, 2024

Conversation

nomeata
Copy link
Contributor

@nomeata nomeata commented Aug 22, 2024

This is part of #3983.

After #4154 introduced equational lemmas for non-recursive functions and #5055
unififed the lemmas for structural and wf recursive funcitons, this now
disables the special handling of recursive functions in
findMatchToSplit?, so that the equational lemmas should be the same no
matter how the function was defined.

The new option eqns.deepRecursiveSplit can be disabled to get the old
behavior.

Breaking change

This can break existing code, as there now can be extra equational
lemmas:

  • Explicit uses of f.eq_2 might have to be adjusted if the numbering
    changed.

  • Uses of rw [f] or simp [f] may no longer apply if they previously
    matched (and introduced a match statement), when the equational
    lemmas got more fine-grained.

    In this case either case analysis on the parameters before rewriting
    helps, or setting the option opt.deepRecursiveSplit false while
    defining the function

@nomeata nomeata changed the title feat: same equational lemmas for recusive and non-recursive functions feat: same equational lemmas for recursive and non-recursive functions Aug 22, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 22, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Aug 22, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Aug 22, 2024
@nomeata
Copy link
Contributor Author

nomeata commented Aug 22, 2024

Srsly, no mathlib adaption needed (besides those for #4154)? If it weren’t for the test case in this PR I’d think I made a mistake. Maybe recursive functions with match in a non-recursive arm are just too obscure.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added builds-mathlib CI has verified that Mathlib builds against this PR and removed breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan labels Aug 22, 2024
@nomeata nomeata added the will-merge-soon …unless someone speaks up label Aug 22, 2024
@nomeata nomeata marked this pull request as ready for review August 22, 2024 13:37
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc August 22, 2024 13:47 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc August 23, 2024 07:56 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc August 24, 2024 10:44 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 24, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Aug 24, 2024
@nomeata nomeata added this pull request to the merge queue Aug 25, 2024
Merged via the queue into master with commit 24f550f Aug 25, 2024
23 checks passed
tobiasgrosser pushed a commit to opencompl/lean4 that referenced this pull request Aug 26, 2024
leanprover#5129)

This is part of leanprover#3983.

After leanprover#4154 introduced equational lemmas for non-recursive functions and
leanprover#5055
unififed the lemmas for structural and wf recursive funcitons, this now
disables the special handling of recursive functions in
`findMatchToSplit?`, so that the equational lemmas should be the same no
matter how the function was defined.

The new option `eqns.deepRecursiveSplit` can be disabled to get the old
behavior.

### Breaking change

This can break existing code, as there now can be extra equational
lemmas:

* Explicit uses of `f.eq_2` might have to be adjusted if the numbering
  changed.

* Uses of `rw [f]` or `simp [f]` may no longer apply if they previously
  matched (and introduced a `match` statement), when the equational
  lemmas got more fine-grained.

  In this case either case analysis on the parameters before rewriting
  helps, or setting the option `opt.deepRecursiveSplit false` while
  defining the function
nomeata added a commit that referenced this pull request Aug 29, 2024
in #4154 and #5129 the rules for equational lemmas have changed, and new
options were introduced that can be used to revert to the pre-4.12
behavior. Hopefully nobody really needs these options besides for
backwards compatibility, therefore we put these options in the
`backward` option name space.

So the previous behavior can be achieved by setting
```lean
set_option backward.eqns.nonrecursive false
set_option backward.eqns.deepRecursiveSplit false
```
github-merge-queue bot pushed a commit that referenced this pull request Aug 29, 2024
in #4154 and #5129 the rules for equational lemmas have changed, and new
options were introduced that can be used to revert to the pre-4.12
behavior. Hopefully nobody really needs these options besides for
backwards compatibility, therefore we put these options in the
`backward` option name space.

So the previous behavior can be achieved by setting
```lean
set_option backward.eqns.nonrecursive false
set_option backward.eqns.deepRecursiveSplit false
```
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 release-ci Enable all CI checks for a PR, like is done for releases toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN will-merge-soon …unless someone speaks up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants