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.(getMsbD, msb)_(extractLsb', extractLsb), getMsbD_extractLsb'_eq_getLsbD #6792

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

luisacicolini
Copy link
Contributor

This PR adds theorems BitVec.(getMsbD, msb)_(extractLsb', extractLsb), getMsbD_extractLsb'_eq_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 Jan 27, 2025
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Jan 27, 2025

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 56733b953eee491cbd1a95018bdb6a76506f5e61 --onto 69a73a18fbfa1fc045bfbf1c4cf93b155d4c9387. (2025-01-27 12:27:39)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 56733b953eee491cbd1a95018bdb6a76506f5e61 --onto 20c616503abe5ce4253c56dbcd7766a91c675ba0. (2025-01-29 13:33:45)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 56733b953eee491cbd1a95018bdb6a76506f5e61 --onto e922edfc218090ab2e54092336c67fe3b970dfc2. (2025-01-30 20:26:30)

@luisacicolini
Copy link
Contributor Author

changelog-library

@github-actions github-actions bot added the changelog-library Library label Jan 28, 2025
Copy link
Contributor

@bollu bollu left a comment

Choose a reason for hiding this comment

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

Thanks for the API :) I request a couple of minor syntactic changes, and one simplification of a theorem statement.

src/Init/Data/BitVec/Lemmas.lean Outdated Show resolved Hide resolved
src/Init/Data/BitVec/Lemmas.lean Outdated Show resolved Hide resolved
Comment on lines 824 to 825
(extractLsb hi lo x).msb = (decide (0 < hi - lo + 1) &&
(decide (lo + (hi - lo) < n) && x.getMsbD (n - 1 - (lo + (hi - lo))))) := by
Copy link
Contributor

Choose a reason for hiding this comment

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

    (extractLsb hi lo x).msb = (decide (0 < hi - lo + 1) &&
    (decide (lo + (hi - lo) < n) && x.getMsbD (n - 1 - (lo + (hi - lo))))) := by

I feel the conditions can be simplified. We should be able to write decide (0 < hi - lo + 1) as decide (lo <= hi). Similarly, given this, is it not the case that decide (lo + (hi - lo) < n) is equivalent to decide (hi < n), and the final case is x.getMsbD (n - 1 - hi)? Note that I'm not 100% sure about the above computations, but it does seem to me that lo + (hi - lo) is refactorable by using another decide (...) condition if necessary

src/Init/Data/BitVec/Lemmas.lean Outdated Show resolved Hide resolved
luisacicolini and others added 3 commits January 29, 2025 11:38
@luisacicolini luisacicolini requested a review from bollu January 29, 2025 14:46
Copy link
Contributor

@alexkeizer alexkeizer left a comment

Choose a reason for hiding this comment

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

Some style comments

src/Init/Data/BitVec/Lemmas.lean 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
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
@luisacicolini
Copy link
Contributor Author

awaiting-review

@github-actions github-actions bot added the awaiting-review Waiting for someone to review the PR label Jan 30, 2025
@luisacicolini luisacicolini marked this pull request as ready for review January 30, 2025 22:52
@luisacicolini luisacicolini requested a review from kim-em as a code owner January 30, 2025 22:52
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 changelog-library Library 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