-
Notifications
You must be signed in to change notification settings - Fork 147
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
rust: Annotates literals with their type. #1671
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the autogenerated files (you should be able to download them from CI on the summary page here for generated-files-bookworm, or you can run make
)
Cargo clippy reports unnecessary literal casting, instead do annotate every literal with its type. This change does not affect any static cast. Extracted rust files are updated. [1] https://rust-lang.github.io/rust-clippy/master/index.html#/unnecessary_cast
37ee69f
to
1a2f5b7
Compare
It looks like one additonal cast must be added in the intermediate representation: For example: fiat_p256_subborrowx_u64(&mut x104, &mut x105, x103, x96, 0xffffffff_u32); With the current patch, every literal has its own type annotation (which corresponds to the bitsize of the value). However, there should be another cast to Previously, there wasn't an issue as the rust compiler automatically converts these constants. I couldn't find where to include this additional cast (note that for C, all constants are casted to u64, when they used as parameters of a function). It seems that this cast must come from the intermediate representation tree. @JasonGross , could you please look into this issue? |
The cast for the third argument should be inserted at fiat-crypto/src/Stringification/IR.v Line 1353 in 37dd5af
fiat-crypto/src/Stringification/IR.v Lines 1040 to 1044 in 37dd5af
C says that this is false , though:fiat-crypto/src/Stringification/C.v Lines 557 to 558 in 37dd5af
while Rust says true fiat-crypto/src/Stringification/Rust.v Lines 382 to 383 in 37dd5af
so I don't see how the casts are getting there in the C code. But continuing on, we have fiat-crypto/src/Stringification/IR.v Lines 566 to 590 in 37dd5af
fiat-crypto/src/Stringification/IR.v Lines 462 to 478 in 37dd5af
I expect Zcast to give usfiat-crypto/src/Stringification/IR.v Line 471 in 37dd5af
so we should be getting a cast at fiat-crypto/src/Stringification/IR.v Line 473 in 37dd5af
I don't have time to dig into this more at the moment, but if you're down to try a bit more adventurous debugging, there are a bunch of debugging examples in fiat-crypto/src/SlowPrimeSynthesisExamples.v Line 422 in 37dd5af
to use Rust rather than C, then you can investigate what's going on. You can either try to do controlled reduction like many of the examples in there do, or you can install https://github.com/coq-community/reduction-effects and do printf-debugging with lazy or cbv by inserting printing effects.
|
Cargo clippy reports unnecessary literal casting, instead do annotate every literal with its type. This change does not affect any static cast.
[1] https://rust-lang.github.io/rust-clippy/master/index.html#/unnecessary_cast