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: ignore no_index around OfNat.ofNat in norm_cast #6438

Merged
merged 1 commit into from
Jan 2, 2025

Conversation

eric-wieser
Copy link
Contributor

@eric-wieser eric-wieser commented Dec 24, 2024

This PR ensures norm_cast doesn't fail to act in the presence of no_index annotations

While #2867 exists, it is necessary to put no_index around OfNat.ofNat in simp lemmas.
This results in extra Expr.mdata nodes, which must be removed before checking for ofNat numerals.

@eric-wieser eric-wieser requested a review from digama0 as a code owner December 24, 2024 00:32
@eric-wieser
Copy link
Contributor Author

This should fix the build error in leanprover-community/mathlib4#20169

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

Mathlib CI status (docs):

mathlib-bors bot pushed a commit to leanprover-community/mathlib4 that referenced this pull request Jan 1, 2025
`ofNat(n)` is a shorthand for `no_index (OfNat.ofNat n)`.

Its purpose is:
* To make it easier to remember to write the `no_index`
* To add a place to put a hoverable docstrings explaining the complexities
* To make it easier to remove the `no_index`es in future if `DiscrTree`s stops needing them around `ofNat` (leanprover/lean4#2867).

This is not exhaustive, but is a step in the right direction.
For now, I only target (some of) the declarations with a `See note [no_index around OfNat.ofNat]`.

Some theorem statements have been corrected in passing.

This exposed two Lean bugs relating to not using `Expr.consumeMData`:

* leanprover/lean4#6438
* leanprover/lean4#6467

These are made more likely by this PR adding `no_index` to the RHS of equalities, but were already possible to trigger by using `simp [<- _]` on existing theorems with `no_index` on the LHS.
@nomeata nomeata added the changelog-language Language features, tactics, and metaprograms label Jan 2, 2025
@nomeata nomeata enabled auto-merge January 2, 2025 09:10
@nomeata nomeata added this pull request to the merge queue Jan 2, 2025
Merged via the queue into leanprover:master with commit 9eb173e Jan 2, 2025
20 of 23 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 changelog-language Language features, tactics, and metaprograms 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.

3 participants