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.(not_sshiftRight, not_sshiftRight_not, getMsb_not, msb_not) #5492

Merged
merged 4 commits into from
Sep 27, 2024

Conversation

luisacicolini
Copy link
Contributor

No description provided.

@luisacicolini luisacicolini marked this pull request as draft September 27, 2024 08:00
Copy link
Contributor

@tobiasgrosser tobiasgrosser left a comment

Choose a reason for hiding this comment

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

LGTM

@luisacicolini luisacicolini marked this pull request as ready for review September 27, 2024 08:05
@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 27, 2024
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 9eef72620448dc267188d954514b4548d909fc52 --onto 0196bca784f82f90b4efd2a85a400daf4ab767f8. (2024-09-27 08:19:14)

@@ -949,6 +949,13 @@ theorem not_not {b : BitVec w} : ~~~(~~~b) = b := by
ext i
simp

@[simp] theorem getMsb_not {x : BitVec w} :
(~~~x).getMsbD i = (decide (i < w) && !(x.getMsbD i)) := by
by_cases h : i < w <;> simp [getMsbD, h] ; omega
Copy link
Contributor

@hargoniX hargoniX Sep 27, 2024

Choose a reason for hiding this comment

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

This proof is a bit unstructured, the <;> simp solves one of the two goals produced by by_cases and then the ; omega only handles the last remaining goal. Usually we do these kinds of proofs like this:

@[simp] theorem not_append {x : BitVec w} {y : BitVec v} : ~~~ (x ++ y) = (~~~ x) ++ (~~~ y) := by
  ext i
  simp only [getLsbD_not, getLsbD_append, cond_eq_if]
  split
  · simp_all
  · simp_all; omega

this is important for proof stability and being able to debug quicker proofs if they ever end up breaking in some way. For example if one of the simp happens to not be powerful enough anymore omega would be thrown at potentially another goal and you'd get a message about it not being able to solve the goal + an open goal still existing etc.

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.

Looks good apart from the small style issue!

@luisacicolini
Copy link
Contributor Author

thanks a lot! I changed the style of the theorem to make it more similar to the one you sent, not sure it's correct style-wise

(~~~x).getMsbD i = (decide (i < w) && !(x.getMsbD i)) := by
simp only [getMsbD]
by_cases h : i < w
· simp_all; omega
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use just simp [h] here. simp_all is a more powerful tool. It basically goes through all hypothesis h_x and does simp [all_hyps_except h_x] at h_x until a fixpoint.

@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 9eef72620448dc267188d954514b4548d909fc52 --onto 0196bca784f82f90b4efd2a85a400daf4ab767f8. (2024-09-27 09:27:38)

@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 9eef72620448dc267188d954514b4548d909fc52 --onto 0196bca784f82f90b4efd2a85a400daf4ab767f8. (2024-09-27 09:39:44)

@hargoniX hargoniX added this pull request to the merge queue Sep 27, 2024
Merged via the queue into leanprover:master with commit 48711ce Sep 27, 2024
13 checks passed
@leanprover-bot leanprover-bot added the P-medium We may work on this issue if we find the time label Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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