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: remove withoutRecover from apply elaboration #5862

Merged
merged 3 commits into from
Oct 28, 2024

Conversation

kmill
Copy link
Collaborator

@kmill kmill commented Oct 28, 2024

The assumptions behind disabling error recovery for the apply tactic no longer seem to hold, since tactic combinators like first themselves disable error recovery when it makes sense.

This addresses part of #3831

Breaking change: elabTermForApply no longer uses withoutRecover. Tactics using elabTermForApply should evaluate whether it makes sense to wrap it with withoutRecover, which is generally speaking when it's used to elaborate identifiers.

The assumptions behind disabling error recovery for the `apply` tactic no longer seem to hold, since tactic combinators like `first` themselves disable error recovery when it makes sense.

This addresses part of leanprover#3831
@kmill kmill requested a review from kim-em as a code owner October 28, 2024 06:32
@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 Oct 28, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Oct 28, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Oct 28, 2024
@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 Oct 28, 2024
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Oct 28, 2024

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Oct 28, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Oct 28, 2024
@leanprover-community-bot leanprover-community-bot 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 Oct 28, 2024
@kmill kmill added this pull request to the merge queue Oct 28, 2024
Merged via the queue into leanprover:master with commit 9eded87 Oct 28, 2024
17 checks passed
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.

2 participants