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

feat: add BitVec.getElem_truncate #5278

Merged
merged 9 commits into from
Sep 16, 2024
Merged

Conversation

tobiasgrosser
Copy link
Contributor

No description provided.

@tobiasgrosser
Copy link
Contributor Author

@semorrison, we modelled this PR after #5247, but I am a bit confused that we would not have getElem on the RHS of these identities. Could you briefly explain why the RHS has getLsbD?

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

Mathlib CI status (docs):

@tobiasgrosser
Copy link
Contributor Author

tobiasgrosser commented Sep 10, 2024

I chatted with @alexkeizer, and his suggestion was to add:

theorem getLsbD_eq_getElem {x : BitVec w} {i : Nat} (h : i < w) :
    x.getLsbD i = x[i] := by
  simp [getLsbD, getElem_eq_testBit_toNat]

to get back to getElem after these rewrites to getLsbD, if the relevant hypothesis is flying around. Does this make sense from your perspective?

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 10, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 10, 2024
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

@leanprover-bot leanprover-bot added the P-high We will work on this issue label Sep 13, 2024
@kim-em
Copy link
Collaborator

kim-em commented Sep 16, 2024

I chatted with @alexkeizer, and his suggestion was to add:

theorem getLsbD_eq_getElem {x : BitVec w} {i : Nat} (h : i < w) :
    x.getLsbD i = x[i] := by
  simp [getLsbD, getElem_eq_testBit_toNat]

to get back to getElem after these rewrites to getLsbD, if the relevant hypothesis is flying around. Does this make sense from your perspective?

Yes, this looks good. I think it can even be a simp lemma.

@kim-em kim-em added the awaiting-author Waiting for PR author to address issues label Sep 16, 2024
@kim-em
Copy link
Collaborator

kim-em commented Sep 16, 2024

Apologies for the delay reviewing here.

@tobiasgrosser
Copy link
Contributor Author

@semorrison, I applied your suggestions. However, I am not use we want to implement them without adding a new hypothesis h : i < n. @alexkeizer's suggestion of introducing getLsbD_eq_getElem was precisely to have the other theorems to be able to have a weaker precondition and a weaker RHS, but allowing for an automatic translation to getElem, whenever a stronger precondition is available.

getLsbD_eq_getElem is not yet @[simp] as this will break the build without a flashed out getElem API. So I will enable it only after the API is complete.

@tobiasgrosser
Copy link
Contributor Author

awaiting-review

@github-actions github-actions bot added awaiting-review Waiting for someone to review the PR and removed awaiting-author Waiting for PR author to address issues labels Sep 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 16, 2024
@tobiasgrosser
Copy link
Contributor Author

I moved this back to getLsbD on the RHS. @semorrison, feel invited to merge.

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 16, 2024
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 16, 2024
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

@kim-em kim-em enabled auto-merge September 16, 2024 08:46
@kim-em kim-em added this pull request to the merge queue Sep 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 16, 2024
Merged via the queue into leanprover:master with commit 7952a7f Sep 16, 2024
14 checks passed
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review Waiting for someone to review the PR builds-mathlib CI has verified that Mathlib builds against this PR P-high We will work on this issue 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.

5 participants