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.intMin #5098

Merged
merged 4 commits into from
Aug 27, 2024
Merged

Conversation

tobiasgrosser
Copy link
Contributor

@tobiasgrosser tobiasgrosser commented Aug 20, 2024

We keep the library interface for intMin minimal and make it an abbrev for twoPow. To ensure a uniform library interface, we also turn intMax into an abbrev for allOnes.

To improve proof automation, we introduce a simp-set boolToPropSimps plus corresponding lemmas that turn our intMin_toLsb proof into a simple simp; omega proof. This simp set is not yet active, but should become so after a stage-0 update.

This PR also pulls in some mathlib theorems on testBit and Nat and establishes facts about 2 ^ w that are needed here and which are generally useful for bitvector reasoning.

The following theorem is not generalized to arbitrary x instead of 2, as this would require a condition to be added for x > 1 which would have to be passed to simp each time this theorem should fire:

protected theorem two_pow_pred_mod_two_pow (h : 0 < w) :
    2 ^ (w - 1) % 2 ^ w = 2 ^ (w - 1)

Mathlib reports only errors for theorems that this PR moves into lean.

error: ././././Mathlib/Data/Nat/Bitwise.lean:250:8: 'Nat.testBit_two_pow_self' has already been declared
error: ././././Mathlib/Data/Nat/Bitwise.lean:254:8: 'Nat.testBit_two_pow_of_ne' has already been declared
error: ././././Mathlib/Data/Nat/Bitwise.lean:265:8: 'Nat.testBit_two_pow' has already been declared

@tobiasgrosser
Copy link
Contributor Author

Draft, as I am still polishing the order of the various theorems within the files. Any feedback is obviously already welcome.

@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 Aug 20, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

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

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase efbecf272dc2c12610efd825e8672cee380b4c24 --onto b486c6748b153bda628575635baa28aeeb38b8c5. (2024-08-20 05:49:30)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase ea97aac83b276bd582471d4d6b5899bac67298fd --onto 2bc87298d9fc5b1a63dd8c10c15e970d5df93fdf. (2024-08-24 20:49:17)
  • 💥 Mathlib branch lean-pr-testing-5098 build failed against this PR. (2024-08-26 11:22:13) View Log
  • 💥 Mathlib branch lean-pr-testing-5098 build failed against this PR. (2024-08-26 13:36:56) View Log
  • 💥 Mathlib branch lean-pr-testing-5098 build failed against this PR. (2024-08-26 14:11:15) View Log
  • 💥 Mathlib branch lean-pr-testing-5098 build failed against this PR. (2024-08-26 14:40:56) View Log
  • 💥 Mathlib branch lean-pr-testing-5098 build failed against this PR. (2024-08-26 15:56:56) View Log
  • 💥 Mathlib branch lean-pr-testing-5098 build failed against this PR. (2024-08-26 16:48:38) View Log
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase c45a6a93f98b004d2e10574d25ee5898bdf0ee34 --onto 9305049f1ef7309842806c2a994a2020bb32a71f. (2024-08-27 01:20:50)

src/Init/Data/BitVec/Lemmas.lean Outdated Show resolved Hide resolved
src/Init/Data/BitVec/Lemmas.lean Outdated Show resolved Hide resolved
Copy link
Contributor

@AtticusKuhn AtticusKuhn 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 to me.

src/Init/Data/BitVec/Lemmas.lean Outdated Show resolved Hide resolved
@tobiasgrosser tobiasgrosser marked this pull request as ready for review August 20, 2024 08:58
@tobiasgrosser tobiasgrosser requested a review from kim-em as a code owner August 20, 2024 08:58
@tobiasgrosser
Copy link
Contributor Author

This is now ready-for-review from my side.

@tobiasgrosser
Copy link
Contributor Author

awaiting-review

@github-actions github-actions bot added the awaiting-review Waiting for someone to review the PR label Aug 20, 2024
Copy link
Contributor

@AnotherAlexHere AnotherAlexHere 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 to me.

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

kim-em commented Aug 26, 2024

As we are modifying simp lemmas for Nat, I would like to see Mathlib tested against this. If you rebase this PR onto nightly-with-mathlib, you should hopefully get a builds-mathlib or breaks-mathlib label within an hour.

@tobiasgrosser tobiasgrosser force-pushed the add_BitVec_intMin branch 3 times, most recently from 4991e87 to 642039d Compare August 26, 2024 09:12
@kim-em kim-em added awaiting-author Waiting for PR author to address issues and removed awaiting-review Waiting for someone to review the PR labels Aug 26, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 26, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Aug 26, 2024
@tobiasgrosser
Copy link
Contributor Author

The comments are adressed, except for finding the correct imports for the new simp set. I will await advice from @bollu.

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 26, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Aug 26, 2024
@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 Aug 26, 2024
@tobiasgrosser
Copy link
Contributor Author

tobiasgrosser commented Aug 26, 2024

The simp-set is also defined, so the remaining issue is a lonely build failure in the nix build, which I am not sure how to debug or if it is even introduced by this PR.

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 26, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Aug 26, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 26, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Aug 26, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 26, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Aug 26, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 26, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Aug 26, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 26, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Aug 26, 2024
@tobiasgrosser
Copy link
Contributor Author

It seems that the intMax definition was incorrect. I corrected it, but I haven't polished the proofs yet.

@kim-em
Copy link
Collaborator

kim-em commented Aug 27, 2024

Remaining steps to activating boolToPropSimps:

  • Move BoolToPropSimps.lean to src/Init/Data/.
  • add import Init.Data.BoolToPropSimps to src/Init/Data.lean
  • commit that
  • make -j32 -C build/release update-stage0
  • git commit -m "chore: update stage0"
    (it's essential the commit message is exactly that)
  • now uncomment the @[boolToPropSimps] attributes, and hopefully you can use simp only [boolToPropSimps].

(If you give me write access on your repo I'm happy to do these steps, too.)

@tobiasgrosser
Copy link
Contributor Author

tobiasgrosser commented Aug 27, 2024

Remaining steps to activating boolToPropSimps:

  • Move BoolToPropSimps.lean to src/Init/Data/.
  • add import Init.Data.BoolToPropSimps to src/Init/Data.lean
  • commit that
  • make -j32 -C build/release update-stage0
  • git commit -m "chore: update stage0"
    (it's essential the commit message is exactly that)
  • now uncomment the @[boolToPropSimps] attributes, and hopefully you can use simp only [boolToPropSimps].

(If you give me write access on your repo I'm happy to do these steps, too.)

Cool. I gave you write access. I guess it's easier if you push this through.

@kim-em kim-em force-pushed the add_BitVec_intMin branch from d0234c7 to 934cc9c Compare August 27, 2024 00:48
tobiasgrosser and others added 4 commits August 27, 2024 11:02
This PR also pulls in some mathlib theorems on testBit and Nat and establishes facts about 2^w that are needed here and which are generally useful for bitvector reasoning.

The following theorem is not generalized to arbitrary x instead of 2, as this would require a condition to be added for x > 1 which would have to be passed to simp each time this theorem should fire.

chore: derive from testBit_two_pow

chore: convert first to prop and then decide

chore: move intMax down as well

chore: add simp set

Add simp-set into this PR

chore: fix simp extension

Move file to src/Lean to fix build

Add prelude

update date

Add university of cambridge as copyright holder

improve naming

use whitespace uniformly

use decide (n = m)

Drop the 'Nat.' namespace

Update src/Init/Data/BitVec/Lemmas.lean

Co-authored-by: Siddharth <[email protected]>

Update src/Init/Data/BitVec/Lemmas.lean

Co-authored-by: Siddharth <[email protected]>

Fix build

add some theorems

Revert "add some theorems"

This reverts commit fb97bc2.

WIP

Shorten proof

Update src/Init/Data/Nat/Lemmas.lean

finish proofs

Update src/Init/Data/BitVec/Lemmas.lean

Co-authored-by: Kim Morrison <[email protected]>

Update src/Init/Data/Nat/Lemmas.lean

Co-authored-by: Kim Morrison <[email protected]>

chore: move BoolToPropSimps
@kim-em kim-em force-pushed the add_BitVec_intMin branch from 6c2e907 to 5f347e7 Compare August 27, 2024 01:05
@kim-em kim-em merged commit 9ef9962 into leanprover:master Aug 27, 2024
13 checks passed
@kim-em kim-em deleted the add_BitVec_intMin branch August 27, 2024 01:26
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 breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan 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.

8 participants