-
Notifications
You must be signed in to change notification settings - Fork 661
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
[RFC] Add initial RFC for type casts. #1056
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1056 +/- ##
==========================================
+ Coverage 75.66% 75.84% +0.17%
==========================================
Files 439 438 -1
Lines 17771 17805 +34
==========================================
+ Hits 13447 13504 +57
+ Misses 4324 4301 -23
Continue to review full report at Codecov.
|
docs/rfc/004-integer-type-casts.md
Outdated
(at least, not for production code). | ||
Presumably, the behavior of type casts in those programming languages | ||
is motivated by efficiency of execution, at least in part. | ||
Since in Leo the input data is avaialable at compile time, |
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.
Typo -> "available". Also, do we want to distinguish between compile-to-IR-or-R1CS time and proof generation time in this discussion, or is that too tangential?
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.
Fixed, thanks.
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.
I think it may be too tangential too at this level. (If you agree, you or I can click the 'Resolve conversation' button here.)
let r_low16: u32 = r & 0xFFFF; // assuming we have bitwise ops and hex literals | ||
let s: u16 = r_low16 as u16; // no value change here | ||
``` | ||
|
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.
The value-preserving casts
alternative does not prevent the later addition of other operators that change the value and type, such as the re-interpretation between signed and unsigned. It might improve clarity of code to have a syntactic difference between value-preserving and value-changing casts.
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.
I agree.
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.
Looks good. How are we going to decide between the alternatives?
Maybe we can start a discussion at this Wednesday's sync. |
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.
LGTM
Closes #1057.