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: .. in patterns should not make use of optparams or autoparams #5933

Merged
merged 1 commit into from
Nov 3, 2024

Conversation

kmill
Copy link
Collaborator

@kmill kmill commented Nov 3, 2024

In patterns, ellipsis should always fill in each remaining argument as an implicit argument, even if it is an optparam or autoparam. This prevents examples such as the one in #4555 from failing:

match e with
| .internal .. => sorry
| .error .. => sorry

The internal constructor has an optparam (| internal (id : InternalExceptionId) (extra : KVMap := {})).

We may consider having ellipsis suppress optparams and autoparams in general. We avoid doing so for now since it's possible to opt-out of them individually (for example with .internal (extra := _) ..) but it's not possible to opt-in, and it is plausible that .. with optparams is useful in contexts such as the refine tactic. With patterns however, it is hard to imagine a use case that offsets the inconvenience of optparams being eagerly supplied.

Closes #4555

In patterns, ellipsis should always be an implicit argument, even if it is an optparam or autoparam. This prevents examples such as the one in leanprover#4555 from failing:
```lean
match e with
| .internal .. => sorry
| .error .. => sorry
```
The `internal` constructor has an optparam (`| internal (id : InternalExceptionId) (extra : KVMap := {})`).

We may consider having ellipsis suppress optparams and autoparams in general. We avoid doing so for now since it's possible to opt-out of them individually (for example with `.internal (extra := _) ..`) but it's not possible to opt-in. While we can opt-out like this for patterns as well, with patterns it is suprising how strongly the optparams are inserted.

Closes leanprover#4555
@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 3, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Nov 3, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 3, 2024
@leanprover-community-bot leanprover-community-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Nov 3, 2024
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

@kmill kmill enabled auto-merge November 3, 2024 18:40
@kmill kmill added this pull request to the merge queue Nov 3, 2024
Merged via the queue into leanprover:master with commit 1659f3b Nov 3, 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.

Failure of exhaustiveness checker when matching on Exception
2 participants