Skip to content

Conversation

@sgraf812
Copy link
Contributor

This PR removes simp annotations for lemmas Id.pure_eq, Id.map_eq and Id.bind_eq because they tend to break the abstraction of computations in Id. In particular, they are problematic for compositional reasoning about do notation.

While a better simp framework is on the horizon with #7352; this PR simply adds the old simp lemmas wherever needed.

Breaking change: Remove the simp annotation from Id.pure_eq,
Id.map_eq and Id.bind_eq. Workaround: add manually to simp set.

@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 Apr 23, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 23, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 23, 2025
@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 Apr 23, 2025
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Apr 23, 2025

Mathlib CI status (docs):

@Rob23oba
Copy link
Contributor

Rob23oba commented Apr 23, 2025

I'm not quite sure what I want to think of this change because:

  1. this PR makes it necessary to explicitly specify Id.pure_eq, Id.bind_eq and Id.map_eq, requiring a large modification to whereever you reason about Id.
  2. fix: replace bad simp lemmas for Id #7352 deprecates exactly these lemmas, so that you need to revert all of the additions again.

I definitely prefer #7352 getting updated to current master and eventually merged but for now I think attribute [-simp] Id.pure_eq Id.bind_eq Id.map_eq whereever needed is a better option than to do this change to core.

This PR removes `simp` annotations for lemmas `Id.pure_eq`, `Id.map_eq`
and `Id.bind_eq` because they tend to break the abstraction of
computations in `Id`. In particular, they are problematic for
compositional reasoning about `do` notation.

While a better `simp` framework is on the horizon with #7352; this PR
simply adds the old simp lemmas wherever needed.

Breaking change: Remove the `simp` annotation from `Id.pure_eq`,
`Id.map_eq` and `Id.bind_eq`. Workaround: add manually to simp set.
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 23, 2025 21:07 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 23, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 23, 2025
@sgraf812
Copy link
Contributor Author

@Rob23oba I agree; I think I'll start from #7352 instead.

@eric-wieser
Copy link
Contributor

@sgraf812, I am very happy for you to take over from #7352. I think the main TODO (other than resolving conflicts) is to think about backwards compatibility and which lemmas should be about Id.run.

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 changelog-library Library 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.

5 participants