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

refactor: WF.fix: use WellFoundedRelation.fixBy #5233

Closed
wants to merge 4 commits into from
Closed

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Sep 2, 2024

this defines

WellFoundedRelation.fixBy {α : Sort u} {β : Sort v} [WellFoundedRelation β] (f : α → β)
  {C : α → Sort w} (F : ∀ x, (∀ y, WellFoundedRelation.rel (f y) (f x) → C y) → C x) (x : α) : C x

based on WellFounded.fix, WellFoundedRelation and InvImage. This
moves some logic from meta code in WF.Fix to this definitions, so mostly
a conceptual simplicification, with the effect that invImage should
no longer show up to the user (even with #5016 disabled.)

this defines
```
WellFoundedRelation.fixBy {α : Sort u} {β : Sort v} [WellFoundedRelation β] (f : α → β)
  {C : α → Sort w} (F : ∀ x, (∀ y, WellFoundedRelation.rel (f y) (f x) → C y) → C x) (x : α) : C x
```
based on `WellFounded.fix`, `WellFoundedRelation` and `InvImage`. This
moves some logic from meta code in WF.Fix to this definitions, so mostly
a conceptual simplicification, with the effect that `invImage` should
no longer show up to the user (even with #5016 disabled.)
@nomeata
Copy link
Collaborator Author

nomeata commented Sep 2, 2024

There is some rather hackish way in which WF.Fix recognizes which termination goals belong to which function in the case of mutual recursion, and this break with this refactoring. I’ll put this on hold until I have a better solution there.

@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 2, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 2, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 2, 2024
@leanprover-community-bot leanprover-community-bot added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Sep 2, 2024
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 7, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 7, 2024
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

@nomeata
Copy link
Collaborator Author

nomeata commented Sep 10, 2024

With #5234 for now off the roadmap, this refactoring isn’t so pressing anymore. Maybe some other time.

@nomeata nomeata closed this Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan 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.

2 participants