-
Notifications
You must be signed in to change notification settings - Fork 167
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
ASR: add UnsignedInteger #1588
Comments
For example, for a: u16 = 1000
b: i16 = -1
a > u16(b) We must decide how |
I believe C just re-uses the bit pattern: https://replit.com/join/jlxoloezhd-brianbeckman |
Yes, C does. Rust, on the other hand, does not allow you to do this: let a: u8 = 100;
let b: i8 = -1;
let val: bool = a > b; which gives: $ cargo run
Compiling xx v0.1.0 (/private/tmp/xx)
error[E0308]: mismatched types
--> src/main.rs:5:25
|
5 | let val: bool = a > b;
| ^ expected `u8`, found `i8`
|
help: you can convert an `i8` to a `u8` and panic if the converted value doesn't fit
|
5 | let val: bool = a > b.try_into().unwrap();
| ++++++++++++++++++++
For more information about this error, try `rustc --explain E0308`.
error: could not compile `xx` due to previous error and this: let a: u8 = 100;
let b: i8 = -1;
let val: bool = a > b.try_into().unwrap(); gives: $ cargo run
Finished dev [unoptimized + debuginfo] target(s) in 0.00s
Running `target/debug/xx`
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: TryFromIntError(())', src/main.rs:5:38
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace So the default way that the rust compiler recommended to convert i8 to u8 is the "safe" method, which gives a runtime error if it does not fit exactly. The casting in Rust is done by
This prints "false". The So Rust allows both methods to convert i8 to u8. Here are more details: https://doc.rust-lang.org/rust-by-example/types/cast.html Conclusion: I think we should cast signed to unsigned like Rust's "as", that is, no change in bits, so |
So, do we want to introduce a new type in ASR like |
Suggested in https://github.com/lcompilers/lpython/wiki/ASR-Design#unsigned-integers #1578 is also one way. However I think |
The best way forward right now seems to be:
First we should implement it, then we should add the Debug time checks for it, to ensure people do not use wrap around. The behavior should be similar to Rust's. |
This is now implemented, see the issue description for the 4 PRs that implemented it. There might still be bugs, but if there are, we'll fix it as they are reported. |
The UnsignedInteger, just like the default (signed) Integer should be checked in Debug and ReleaseSafe mode for overflow (from both sides). In Release mode they are not checked for performance.
We have to carefully check that this design fixes all the pitfalls at https://github.com/lcompilers/lpython/wiki/ASR-Design#unsigned-integers, but it looks like it might.
If wraparound is required, then the design at #1578 should be used.
Implementation:
The text was updated successfully, but these errors were encountered: