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 elabAsElim aware of explicit motive arguments #4817

Merged
merged 2 commits into from
Jul 29, 2024

Conversation

kmill
Copy link
Collaborator

@kmill kmill commented Jul 24, 2024

Some eliminators (such as False.rec) have an explicit motive argument. The elabAsElim elaborator assumed that all motives are implicit.

If the explicit motive argument is _, then it uses the elab-as-elim procedure, and otherwise it falls back to the standard app elaborator.

Furthermore, if an explicit elaborator is not provided, it falls back to treating the elaborator as being implicit, which is convenient for writing h.rec rather than h.rec _. Rationale: for False.rec, this simulates it having an implicit motive, and also motives are generally not going to be available in the expected type.

Closes #4347

@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 Jul 24, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jul 24, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jul 24, 2024
@nomeata
Copy link
Collaborator

nomeata commented Jul 24, 2024

Just based off the PR description: did you consider the alternative of making all motives implicit? Or is there a good reason to make them explicit it some cases?

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan release-ci Enable all CI checks for a PR, like is done for releases labels Jul 24, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jul 24, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jul 24, 2024
@kmill
Copy link
Collaborator Author

kmill commented Jul 24, 2024

@nomeata Yes, and that's a good question that's worth discussing here. First, there's Leo's comment about how eliminators like False.rec come from the kernel, and so we should fix the elaborator. Second, user eliminators do sometimes have explicit motives for whatever reason.

@nomeata
Copy link
Collaborator

nomeata commented Jul 24, 2024

Thanks, these are good points. Ah, and I should have followed the link to the issue to find it out myself :-)

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Jul 24, 2024
@kmill kmill enabled auto-merge July 27, 2024 17:36
@kmill kmill added this pull request to the merge queue Jul 27, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 27, 2024
kmill added 2 commits July 29, 2024 09:45
Some eliminators (such as `False.rec`) have an explicit motive argument. The `elabAsElim` elaborator assumed that all motives are implicit.

Closes leanprover#4347
@kmill kmill force-pushed the elabaselim_explicit branch from 3d2d63d to 5415c61 Compare July 29, 2024 18:27
@kmill kmill enabled auto-merge July 29, 2024 18:29
@kmill kmill added this pull request to the merge queue Jul 29, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jul 29, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jul 29, 2024
Merged via the queue into leanprover:master with commit 69c71f6 Jul 29, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-ci Enable all CI checks for a PR, like is done for releases toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
3 participants