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: don't use modulo for UInt upcasting #3960

Merged
merged 1 commit into from
Apr 20, 2024

Conversation

digama0
Copy link
Collaborator

@digama0 digama0 commented Apr 20, 2024

This makes (v : UInt8).toUInt16.toNat = (v : UInt8).toNat a defeq, which simplifies proofs.

@nomeata nomeata enabled auto-merge April 20, 2024 15:46
@nomeata nomeata added this pull request to the merge queue Apr 20, 2024
@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 Apr 20, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 20, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 20, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Apr 20, 2024

Mathlib CI status (docs):

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Apr 20, 2024
@nomeata nomeata removed this pull request from the merge queue due to a manual request Apr 20, 2024
@nomeata
Copy link
Collaborator

nomeata commented Apr 20, 2024

(Removed from queue due to mathlib failures; were they expected?)

@digama0
Copy link
Collaborator Author

digama0 commented Apr 20, 2024

Yes, they are expected. Should I make a bump PR? The breakage should be minimal, because mathlib doesn't do much with fixed size integers.

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 20, 2024
@digama0
Copy link
Collaborator Author

digama0 commented Apr 20, 2024

Just checked, it is from theorems in Std like this:

@[simp] theorem UInt8.toUInt16_toNat (x : UInt8) : x.toUInt16.toNat = x.toNat :=
  Nat.mod_eq_of_lt (Nat.lt_of_lt_of_le x.toNat_lt (by decide))

@[simp] theorem UInt8.toUInt32_toNat (x : UInt8) : x.toUInt32.toNat = x.toNat :=
  Nat.mod_eq_of_lt (Nat.lt_of_lt_of_le x.toNat_lt (by decide))

@[simp] theorem UInt8.toUInt64_toNat (x : UInt8) : x.toUInt64.toNat = x.toNat :=
  Nat.mod_eq_of_lt (Nat.lt_of_lt_of_le x.toNat_lt (by decide))

which are exactly the ones that are rfl after this PR. So it should be easily fixable.

leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 20, 2024
@nomeata
Copy link
Collaborator

nomeata commented Apr 20, 2024

If you say it's fine I can merge, although it doesn't hurt to have the fixes ready ahead of time, by pushing to the lean-pr-testing branches. Then whoever updates nightly-testing has less to think about, don't they?

@nomeata nomeata added this pull request to the merge queue Apr 20, 2024
Merged via the queue into leanprover:master with commit 5eb274d Apr 20, 2024
18 checks passed
@kim-em
Copy link
Collaborator

kim-em commented Apr 21, 2024

I did encounter the errors on Std's nightly-testing before reading this thread, but found it pretty quickly.

I think a good option, when not having a lean-pr-testing-NNNN branch ready (or even when!) is to post on zulip's nightly-testing stream saying that a breakage is expected (and, if available, a quick description of the fix).

@digama0 digama0 deleted the uint_upcast branch April 22, 2024 02:48
github-merge-queue bot pushed a commit that referenced this pull request Apr 22, 2024
- [x] Depends on: #3958 
- [x] Depends on: #3960

This makes the UTF-8 encode and decode functions have lean definitions,
so that we can prove properties about them downstream.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan 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.

4 participants