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: deriving DecidableEq to use termination_by structural #4826

Merged
merged 3 commits into from
Jul 29, 2024

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Jul 25, 2024

now that we support structural mutual recursion, I expect that every
DecidableEq instance be implemented using structural recursion, so
let's be explicit about it.

now that we support structural mutual recursion, I expect that every
`DecidableEq` instance be implemented using structural recursion, so
let's be explicit about it.
@nomeata nomeata requested a review from kim-em as a code owner July 25, 2024 08:03
@nomeata nomeata added the awaiting-review Waiting for someone to review the PR label Jul 25, 2024
@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 Jul 25, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc July 25, 2024 10:14 Inactive
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Jul 25, 2024

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 93fa9c8837e071a866b62e2ca73c22a362ccb988 --onto 5938dbbd14145c9b78f7645c4777f62a3fc8212c. (2024-07-25 10:18:58)
  • ✅ Mathlib branch lean-pr-testing-4826 has successfully built against this PR. (2024-07-29 15:24:55) View Log

@nomeata
Copy link
Collaborator Author

nomeata commented Jul 25, 2024

@arthur-adjedj, what do you think about this change?

@arthur-adjedj
Copy link
Contributor

arthur-adjedj commented Jul 25, 2024

I think ensuring good reduction on decide is desirable, so I'm all for this. Some other derive handlers such as BEq and Hashable might also benefit from having them forcibly be structural, but I'm not so certain it would be useful enough to impose it.

@nomeata
Copy link
Collaborator Author

nomeata commented Jul 25, 2024

I guess I’ll wait until we have a nighty-with-mathlib and see if it survives testing against mathlib :-)

@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc July 29, 2024 14:18 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jul 29, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jul 29, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Jul 29, 2024
@nomeata nomeata added this pull request to the merge queue Jul 29, 2024
@nomeata nomeata removed the awaiting-review Waiting for someone to review the PR label Jul 29, 2024
Merged via the queue into master with commit 4ea5568 Jul 29, 2024
17 checks passed
@nomeata nomeata deleted the joachim/decEq-structural branch July 29, 2024 22:14
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.

3 participants