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: tactics in terms in tactics may break incremental reporting #4436

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

Kha
Copy link
Member

@Kha Kha commented Jun 12, 2024

A pending tactic mvar managed to escape into an unexpected context in specific circumstances.

example : True := by
  · rw [show 0 = 0 by rfl]
  • Term elaboration of the show creates a pending mvar for the by rfl proof
  • rw fails with an exception because the pattern does not occur in the target
  • cdot catches the exception and admits the goal
  • Term.runTactic synthesizes all pending mvars from the tactic's execution, including the by rfl proof. But this would not have happened without cdot as the exception would have skipped that invocation!
  • Now incrementality is confused because the nested by rfl proof is unexpectedly run in the same context as the top-level proof, writing to the wrong promise, and the error message is lost

Solution: disable incrementality for these pending mvars

@Kha Kha enabled auto-merge June 12, 2024 14:49
@Kha Kha added this pull request to the merge queue Jun 12, 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 Jun 12, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 5f9dedfe5ee9972acdebd669f228f487844a6156 --onto ea46bf2839ad1c98d3a0c3e5caad8a81f812934c. (2024-06-12 15:01:56)

Merged via the queue into leanprover:master with commit 8d3be96 Jun 12, 2024
14 checks passed
github-actions bot pushed a commit that referenced this pull request Jun 12, 2024
A pending tactic mvar managed to escape into an unexpected context in
specific circumstances.

```lean
example : True := by
  · rw [show 0 = 0 by rfl]
```
* Term elaboration of the `show` creates a pending mvar for the `by rfl`
proof
* `rw` fails with an exception because the pattern does not occur in the
target
* `cdot` catches the exception and admits the goal
* `Term.runTactic` [synthesizes all pending mvars from the tactic's
execution](https://github.com/leanprover/lean4/blob/5f9dedfe5ee9972acdebd669f228f487844a6156/src/Lean/Elab/SyntheticMVars.lean#L350),
including the `by rfl` proof. But this would not have happened without
`cdot` as the exception would have skipped that invocation!
* Now incrementality is confused because the nested `by rfl` proof is
unexpectedly run in the same context as the top-level proof, writing to
the wrong promise, and the error message is lost

Solution: disable incrementality for these pending mvars
(cherry picked from commit 8d3be96)
Kha added a commit that referenced this pull request Jun 12, 2024
A pending tactic mvar managed to escape into an unexpected context in
specific circumstances.

```lean
example : True := by
  · rw [show 0 = 0 by rfl]
```
* Term elaboration of the `show` creates a pending mvar for the `by rfl`
proof
* `rw` fails with an exception because the pattern does not occur in the
target
* `cdot` catches the exception and admits the goal
* `Term.runTactic` [synthesizes all pending mvars from the tactic's
execution](https://github.com/leanprover/lean4/blob/5f9dedfe5ee9972acdebd669f228f487844a6156/src/Lean/Elab/SyntheticMVars.lean#L350),
including the `by rfl` proof. But this would not have happened without
`cdot` as the exception would have skipped that invocation!
* Now incrementality is confused because the nested `by rfl` proof is
unexpectedly run in the same context as the top-level proof, writing to
the wrong promise, and the error message is lost

Solution: disable incrementality for these pending mvars
(cherry picked from commit 8d3be96)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport releases/v4.9.0 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