Skip to content

Conversation

@abentkamp
Copy link
Contributor

While working on verifying the GCD crate, I need to add these definitions and lemmas to the proof-libs.

An open question: Is there a way to generalize over all instances of shift_right_trailing_zeros_nonzero_lemma as I have managed to do for shift_right_le?

And generally, I am not sure if we want these kinds of specific lemmas in the proof-libs.

@abentkamp abentkamp requested a review from a team as a code owner November 6, 2025 10:17
@abentkamp abentkamp requested a review from W95Psp November 6, 2025 10:17
@abentkamp abentkamp force-pushed the alex/haxlib-additions-gcd branch from dcfc5ea to 3158389 Compare November 10, 2025 09:23
@maximebuyse
Copy link
Contributor

maximebuyse commented Nov 10, 2025

core::cmp and core::num are already switched to Rust definitions from core models in hax-evit. We shouldn't modify the hand-written F* core lib at this point. @abentkamp could you do a PR to hax-evit instead with modifications to the rust models (and F* rust_primitives)?

Comment on lines +122 to +127
val impl_u8__trailing_zeros: u8 -> shiftval U8 U32
val impl_u16__trailing_zeros: u16 -> shiftval U16 U32
val impl_u32__trailing_zeros: u32 -> shiftval U32 U32
val impl_u64__trailing_zeros: u64 -> shiftval U64 U32
val impl_u128__trailing_zeros: u128 -> shiftval U128 U32
val impl_usize__trailing_zeros: usize -> shiftval USIZE U32
Copy link
Member

Choose a reason for hiding this comment

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

I think shiftval _ U32 is incorrect: 0u8 has 8 trailing zeros, but 8u32 doesn't fit in shiftval U8 U32.
Indeed, shiftval U8 U32 is defined as a number smaller than bits U8, which is 8.

So here, you can replace shiftval _ _ by (n:u32 {n <= bits U8})

Copy link
Member

Choose a reason for hiding this comment

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

(I meant using shiftval as a return type here, not the definition of shiftval)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, good catch. Anyway, I'll have to do this differently in the Rust core models, I think.

Comment on lines +344 to +348
val shift_right_le (#t:inttype) (#t':inttype)
(a:int_t t) (b:shiftval t t'):
Lemma (requires (unsigned t \/ v a >= 0))
(ensures (v (shift_right #t #t' a b) <= v a))
[SMTPat (shift_right #t #t' a b)]
Copy link
Member

Choose a reason for hiding this comment

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

Is this lemma actually required?
It should really follow automatically from shift_right_lemma 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked into this some more. Here is what is missing from making it work via shift_right_lemma:

val div_le
    (a:nat) (b:nat):
    Lemma (requires (b > 0))
          (ensures (a / b <= a))
          [SMTPat (a / b)]    

@W95Psp
Copy link
Member

W95Psp commented Nov 10, 2025

On the F* side, you could define:

val trailing_zeros: #t:inttype -> int_t t -> (n:u32{v n >= 0 /\ v n <= bits t})

unfold let impl_u8__trailing_zeros (n:u8) = trailing_zeros n
unfold let impl_u16__trailing_zeros (n:u16) = trailing_zeros n
unfold let impl_u32__trailing_zeros (n:u32) = trailing_zeros n
unfold let impl_u64__trailing_zeros (n:u64) = trailing_zeros n
unfold let impl_u128__trailing_zeros (n:u128) = trailing_zeros n
unfold let impl_usize__trailing_zeros (n:usize) = trailing_zeros n

val shift_right_trailing_zeros_nonzero_lemma #t (a: int_t t):
    Lemma (requires (v a <> 0))
          (ensures (v (shift_right a (trailing_zeros a)) <> 0))
          [SMTPat (shift_right a (trailing_zeros a))]

But not clear how this fits the Rust core models story, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants