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

fix: replace bad simp lemmas for Id #7352

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eric-wieser
Copy link
Contributor

@eric-wieser eric-wieser commented Mar 5, 2025

This PR reworks the simp set around the Id monad.

In particular, it stops encoding the "defeq abuse" of Id X = X in the statements of theorems, instead using Id.run and pure to pass back and forth between these two spellings.

This fixes the problem with the current simp set where Id.run (pure x) is simplified to Id.run x, instead of the desirable x.
This is particularly bad because the x is sometimes inferred with type Id X instead of X, which prevents other simp lemmas about X from firing.

Making Id reducible instead is not an option, as then the Monad instances would have nothing to key on.

@eric-wieser
Copy link
Contributor Author

changelog-library

@github-actions github-actions bot added the changelog-library Library label Mar 5, 2025
@eric-wieser eric-wieser requested review from TwoFX and tydeu as code owners March 6, 2025 01:26
@eric-wieser eric-wieser marked this pull request as draft March 6, 2025 01:48
@eric-wieser
Copy link
Contributor Author

I've split some helper lemmas to #7356, which are independent of this fix

@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 Mar 6, 2025
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Mar 6, 2025

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 9ae2ac39c9512461643e268f0feff5654044776f --onto 5536281238dff2fb4e0a54da472d4f0d6496069e. You can force Mathlib CI using the force-mathlib-ci label. (2025-03-06 03:18:31)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase e0fa6a1792458af81c8f421f5e75bfbc4117f617 --onto 7bfa8f6296ebfebc497d5b30d34f494bcc9782a2. You can force Mathlib CI using the force-mathlib-ci label. (2025-03-10 23:19:19)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase e0fa6a1792458af81c8f421f5e75bfbc4117f617 --onto 8fc8e8ed19ef218022f5a94cbf5e472e3b777e44. You can force Mathlib CI using the force-mathlib-ci label. (2025-03-11 23:53:26)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase d538e1cd90bc8dce3e8919d2ed496903577a63a9 --onto c1d145e9d70569274ac868805b4a8591d09718af. You can force Mathlib CI using the force-mathlib-ci label. (2025-03-14 01:21:44)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 5c333d88c01f71180f1ceeb00fcdad2bea1bdbf9 --onto c1d145e9d70569274ac868805b4a8591d09718af. You can force Mathlib CI using the force-mathlib-ci label. (2025-03-14 01:50:23)

@eric-wieser eric-wieser force-pushed the patch-43 branch 2 times, most recently from 729e005 to 439669c Compare March 10, 2025 22:51
github-merge-queue bot pushed a commit that referenced this pull request Mar 14, 2025
This PR makes `simp` able to simplify basic `for` loops in monads other
than `Id`.

This is some prework for #7352, where the `Id` lemmas will be
deprecated.
This is free at runtime, since both `Id.run` and `pure` are inlined.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

2 participants