Skip to content

fix: make sure monad lift coercion elaborator has no side effects#6024

Merged
kmill merged 3 commits intoleanprover:masterfrom
kmill:fix_coercemonadlift
Nov 13, 2024
Merged

fix: make sure monad lift coercion elaborator has no side effects#6024
kmill merged 3 commits intoleanprover:masterfrom
kmill:fix_coercemonadlift

Conversation

@kmill
Copy link
Copy Markdown
Collaborator

@kmill kmill commented Nov 10, 2024

This PR fixes a bug where the monad lift coercion elaborator would partially unify expressions even if they were not monads. This could be taken advantage of to propagate information that could help elaboration make progress, for example the first change worked because the monad lift coercion elaborator was unifying @Eq _ _ with @Eq (Nat × Nat) p:

example (p : Nat × Nat) : p = p := by
  change _ = ⟨_, _⟩ -- used to work (yielding `p = (p.fst, p.snd)`), now it doesn't
  change ⟨_, _⟩ = _ -- never worked

As such, this is a breaking change; you may need to adjust expressions to include additional implicit arguments.

@kmill kmill added the changelog-language Language features and metaprograms label Nov 10, 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 Nov 10, 2024
ghost pushed a commit to leanprover-community/batteries that referenced this pull request Nov 10, 2024
ghost pushed a commit to leanprover-community/mathlib4 that referenced this pull request Nov 10, 2024
@ghost ghost added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Nov 10, 2024
ghost pushed a commit to leanprover-community/batteries that referenced this pull request Nov 11, 2024
ghost pushed a commit to leanprover-community/mathlib4 that referenced this pull request Nov 11, 2024
kim-em added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 11, 2024
kim-em added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 11, 2024
@ghost ghost 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 Nov 11, 2024
kim-em added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 12, 2024
kim-em added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 12, 2024
mathlib-bors bot pushed a commit to leanprover-community/mathlib4 that referenced this pull request Nov 12, 2024
leanprover/lean4#6024 fixes a serious elaboration bug which, perversely, was quite helpful.

Kyle is investigating replacing the bug with something intentional, but we definitively need to fix the bug in the meantime.

This is the backport of changes from lean-pr-testing-6024 which do not have conflicts with `master`. There are a few more changes that we'll need to handle separately later.

Co-authored-by: Kyle Miller <kmill31415@gmail.com>
@kmill kmill added this pull request to the merge queue Nov 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 13, 2024
This PR fixes a bug where the monad lift coercion elaborator would partially unify expressions even if they were not monads. Breaking change: examples such as `change _ = .some true` no longer work. They accidentally worked because this elaborator left an accidental type hint on `Eq`.
@kmill kmill force-pushed the fix_coercemonadlift branch from 2b0f9bb to 53e20fe Compare November 13, 2024 04:02
@kmill kmill enabled auto-merge November 13, 2024 04:02
@kmill kmill added this pull request to the merge queue Nov 13, 2024
@ghost
Copy link
Copy Markdown

ghost commented Nov 13, 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 1315266dd3faeaf28d1263668cb88f2f3f5e530c --onto 5e01e628b2ef90d8881a5ba10340032eeeabc5d4. (2024-11-13 04:21:42)

@kmill kmill removed this pull request from the merge queue due to a manual request Nov 13, 2024
@kmill kmill added this pull request to the merge queue Nov 13, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 13, 2024
@kmill kmill requested a review from kim-em as a code owner November 13, 2024 06:40
@kmill kmill enabled auto-merge November 13, 2024 06:40
@kmill kmill added this pull request to the merge queue Nov 13, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Nov 13, 2024
@kmill kmill added this pull request to the merge queue Nov 13, 2024
@kmill kmill removed this pull request from the merge queue due to a manual request Nov 13, 2024
@kmill kmill added this pull request to the merge queue Nov 13, 2024
Merged via the queue into leanprover:master with commit 28cf146 Nov 13, 2024
TobiasLeichtfried pushed a commit to leanprover-community/mathlib4 that referenced this pull request Nov 21, 2024
leanprover/lean4#6024 fixes a serious elaboration bug which, perversely, was quite helpful.

Kyle is investigating replacing the bug with something intentional, but we definitively need to fix the bug in the meantime.

This is the backport of changes from lean-pr-testing-6024 which do not have conflicts with `master`. There are a few more changes that we'll need to handle separately later.

Co-authored-by: Kyle Miller <kmill31415@gmail.com>
JovanGerb pushed a commit to JovanGerb/lean4 that referenced this pull request Jan 21, 2025
…anprover#6024)

This PR fixes a bug where the monad lift coercion elaborator would
partially unify expressions even if they were not monads. This could be
taken advantage of to propagate information that could help elaboration
make progress, for example the first `change` worked because the monad
lift coercion elaborator was unifying `@Eq _ _` with `@Eq (Nat × Nat)
p`:
```lean
example (p : Nat × Nat) : p = p := by
  change _ = ⟨_, _⟩ -- used to work (yielding `p = (p.fst, p.snd)`), now it doesn't
  change ⟨_, _⟩ = _ -- never worked
```
As such, this is a breaking change; you may need to adjust expressions
to include additional implicit arguments.
mathlib-bors bot pushed a commit to leanprover-community/mathlib4 that referenced this pull request Jul 22, 2025
We have some adaptation notes referring to [lean4#6024](leanprover/lean4#6024), which changed elaboration subtly; some tricky unifications no longer get solved. I checked every reference to make sure that these still apply. We don't expect that this change gets reverted, so we can turn the remaining adaptation notes into regular comments.
callesonne pushed a commit to callesonne/mathlib4 that referenced this pull request Jul 24, 2025
…unity#27348)

We have some adaptation notes referring to [lean4#6024](leanprover/lean4#6024), which changed elaboration subtly; some tricky unifications no longer get solved. I checked every reference to make sure that these still apply. We don't expect that this change gets reverted, so we can turn the remaining adaptation notes into regular comments.
hrmacbeth pushed a commit to szqzs/mathlib4 that referenced this pull request Jul 28, 2025
…unity#27348)

We have some adaptation notes referring to [lean4#6024](leanprover/lean4#6024), which changed elaboration subtly; some tricky unifications no longer get solved. I checked every reference to make sure that these still apply. We don't expect that this change gets reverted, so we can turn the remaining adaptation notes into regular comments.
Equilibris pushed a commit to Equilibris/mathlib4 that referenced this pull request Aug 7, 2025
…unity#27348)

We have some adaptation notes referring to [lean4#6024](leanprover/lean4#6024), which changed elaboration subtly; some tricky unifications no longer get solved. I checked every reference to make sure that these still apply. We don't expect that this change gets reverted, so we can turn the remaining adaptation notes into regular comments.
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 changelog-language Language features and metaprograms 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.

1 participant