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: make elabTermEnsuringType respect errToSorry when there is a type mismatch #3633

Merged
merged 1 commit into from
Mar 9, 2024

Conversation

kmill
Copy link
Collaborator

@kmill kmill commented Mar 7, 2024

Floris van Doorn reported on Zulip that it is confusing that the have : T := e tactic completely fails if the body e is not of type T. This is in contrast to have : T := by exact e, which does not completely fail when e is not of type T.

This ends up being caused by elabTermEnsuringType throwing an error when it fails to insert a coercion. Now, it detects this case, and it checks the errToSorry flag to decide whether to throw the error or to log the error and insert a sorry.

This is justified by elabTermEnsuringType being a frontend to elabTerm, which inserts sorry on error.

An alternative would be to make ensureType respect errToSorry, but there exists code that expects being able to catch when ensureType fails. Making such code manipulate errToSorry seems error prone, and this function is not a main entry point to the term elaborator, unlike elabTermEnsuringType.

@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 7, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Mar 7, 2024

Mathlib CI status (docs):

  • ❗ Std/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 0072d13bd40aef791090d1e3e964413f1176162e --onto ae492265fec103aa834d897bf9f68c94d10f0785. (2024-03-07 20:12:32)
  • ✅ Mathlib branch lean-pr-testing-3633 has successfully built against this PR. (2024-03-07 21:50:04) View Log
  • ✅ Mathlib branch lean-pr-testing-3633 has successfully built against this PR. (2024-03-07 22:29:22) View Log

… type mismatch

Floris van Doorn [reported on Zulip](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/have.20tactic.20error.20recovery/near/425283053) that it is confusing that the `have : T := e` tactic completely fails if the body `e` is not of type `T`. This is in contrast to `have : T := by exact e`, which does not completely fail when `e` fails to have type `T`.

This ends up being caused by `elabTermEnsuringType` throwing an error when it fails inserting a coercion. Now, it detects this case, and it checks the `errToSorry` flag to decide whether to throw the error or to log the error and insert a `sorry`.

This is justified by `elabTermEnsuringType` being a frontend to `elabTerm`, which inserts `sorry` on error.

An alternative would be to make `ensureType` respect `errToSorry`, but there exists code that catch errors from this function.
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 7, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 7, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Mar 7, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 7, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 7, 2024
@leodemoura leodemoura added this pull request to the merge queue Mar 9, 2024
Merged via the queue into leanprover:master with commit 3acd77a Mar 9, 2024
18 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.

None yet

3 participants