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: BitVec.signExtend.{toNat, toInt} #6038

Closed
wants to merge 4 commits into from

Conversation

mhk119
Copy link
Contributor

@mhk119 mhk119 commented Nov 11, 2024

This PR adds toNat and toInt theorems for BitVec.signExtend.

@mhk119 mhk119 requested a review from kim-em as a code owner November 11, 2024 18:06
@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 11, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Nov 11, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Nov 11, 2024
@leanprover-community-bot leanprover-community-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Nov 11, 2024
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Nov 11, 2024

Mathlib CI status (docs):

  • ✅ Mathlib branch lean-pr-testing-6038 has successfully built against this PR. (2024-11-11 19:30:53) View Log
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 91c14c7ee912d3d9b86ad8655098bac67e919df7 --onto 4600bb16fcded0356d20ae232e7f8580c56a5955. (2024-11-19 20:42:10)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 9cf83706e743debb47f3b1a48e1b92210c1c0720 --onto 72e952eadc6a171310f1d8e9d6e78acf98421494. (2024-11-21 16:01:37)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 27cc0c80399f9f5ecebfdff65169d217a3045eea --onto 6d495586a1836da3e12efb4fbf9946bd9088ac5f. (2024-11-29 13:21:48)

@leanprover-bot leanprover-bot added the P-medium We may work on this issue if we find the time label Nov 15, 2024
Copy link
Contributor

@hargoniX hargoniX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor remarks, I got two additional ones:

  1. Could you rebase your branch on leanprover master? The test task IO flipper should be gone there.
  2. Did you check that your theorem statements are in simp normal form?

src/Init/Data/BitVec/Lemmas.lean Outdated Show resolved Hide resolved
src/Init/Data/BitVec/Lemmas.lean Outdated Show resolved Hide resolved
src/Init/Data/BitVec/Lemmas.lean Outdated Show resolved Hide resolved
@hargoniX hargoniX added the awaiting-author Waiting for PR author to address issues label Nov 16, 2024
@hargoniX hargoniX removed the request for review from kim-em November 16, 2024 17:08
@bollu bollu force-pushed the signExtend branch 2 times, most recently from 28e29f4 to 1baa298 Compare November 21, 2024 15:33
This PR adds toNat and toInt theorems for BitVec.signExtend.

Co-authored-by: Siddharth Bhat <[email protected]>
Co-authored-by: Harun Khan <[email protected]>
@mhk119 mhk119 requested a review from hargoniX November 29, 2024 12:55
@hargoniX
Copy link
Contributor

The PR now just seems to consist of a change to code that is fine according to the style guide? What is the motivation here?

@mhk119
Copy link
Contributor Author

mhk119 commented Dec 1, 2024

The PR now just seems to consist of a change to code that is fine according to the style guide? What is the motivation here?

My bad, I didn't see that the theorems got merged into main in the meantime. Closing this :).

@mhk119 mhk119 closed this Dec 1, 2024
@tobiasgrosser
Copy link
Contributor

It seems these got added in #6155 and #6157. In the future, we should cross-reference commits when someone decides to split them. Thank you @mhk119 for getting this in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-author Waiting for PR author to address issues builds-mathlib CI has verified that Mathlib builds against this PR P-medium We may work on this issue if we find the time 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