-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Should const-evaluation be dynamically-typed or statically-typed? (And with what types?) #1071
Comments
(I consider rust-lang/rust#15270 to be a specific instance of this, so I am not planning to clone that issue.) |
potentially related recent PR attempt: rust-lang/rust#18538 |
My first thought here would be that constant evaluation should never differ from runtime evaluation in this regard, that is, a constant expression should be accepted by the type checker iff its runtime equivalent is. |
"iff" may be a bit stronger than I would like. There may be expressions we want to accept at runtime because they do interesting things that have to do with the target architecture -- but those same expressions are likely to be very difficult to make any sense of at compile time. I'm thinking in particular of integer-to-pointer casts, where the original integer was computed solely from literal inputs, and not an address-computation. Hypothetically the const-evaluator could carry such info around as the "dynamic type" of the computed value ... i.e. the value itself could be an LLVM-valueref (and thus may not have access to its integer value), or it could be an integer we have computed, but either way, the (dynamic) type (or "tag", if you prefer) would include information about whether this computed value can be sanely converted to a pointer. See also rust-lang/rust#18294 and perhaps more importantly, rust-lang/rust#24581 or i guess you clearly prefer rust-lang/rust#13973 ;) |
in case it's dynamically typed
in case it's statically typed
actually I think @jakub-'s statement is correct: not all runtime expressions are allowed as constant expressions, but all constant expressions should behave as if they were computed at runtime. |
@oli-obk perhaps we are in violent agreement? I was responding to this proposed rule: "a constant expression should be accepted by the type checker iff its runtime equivalent is." Which obviously conflicts with "not all runtime expressions are allowed as constant expressions" |
I actually have a PR in the works that will end all those type confusions forever. I added variants for every builtin integral type and a signed + and unsigned inference type. I also added a usize and a isize enum that have 32 bit and 64 bit variants and check the tcx for the target architecture during creation and various const eval steps to ensure the proper bit-size and to ensure I didn't screw up somewhere in the middle. So far I have not introduced any breaking changes (except for better error reporting in some locations). |
typestrong const integers ~~It would be great if someone could run crater on this PR, as this has a high danger of breaking valid code~~ Crater ran. Good to go. ---- So this PR does a few things: 1. ~~const eval array values when const evaluating an array expression~~ 2. ~~const eval repeat value when const evaluating a repeat expression~~ 3. ~~const eval all struct and tuple fields when evaluating a struct/tuple expression~~ 4. remove the `ConstVal::Int` and `ConstVal::Uint` variants and replace them with a single enum (`ConstInt`) which has variants for all integral types * `usize`/`isize` are also enums with variants for 32 and 64 bit. At creation and various usage steps there are assertions in place checking if the target bitwidth matches with the chosen enum variant 5. enum discriminants (`ty::Disr`) are now `ConstInt` 6. trans has its own `Disr` type now (newtype around `u64`) This obviously can't be done without breaking changes (the ones that are noticable in stable) We could probably write lints that find those situations and error on it for a cycle or two. But then again, those situations are rare and really bugs imo anyway: ```rust let v10 = 10 as i8; let v4 = 4 as isize; assert_eq!(v10 << v4 as usize, 160 as i8); ``` stops compiling because 160 is not a valid i8 ```rust struct S<T, S> { a: T, b: u8, c: S } let s = S { a: 0xff_ff_ff_ffu32, b: 1, c: 0xaa_aa_aa_aa as i32 }; ``` stops compiling because `0xaa_aa_aa_aa` is not a valid i32 ---- cc @eddyb @pnkfelix related: rust-lang/rfcs#1071
typestrong const integers ~~It would be great if someone could run crater on this PR, as this has a high danger of breaking valid code~~ Crater ran. Good to go. ---- So this PR does a few things: 1. ~~const eval array values when const evaluating an array expression~~ 2. ~~const eval repeat value when const evaluating a repeat expression~~ 3. ~~const eval all struct and tuple fields when evaluating a struct/tuple expression~~ 4. remove the `ConstVal::Int` and `ConstVal::Uint` variants and replace them with a single enum (`ConstInt`) which has variants for all integral types * `usize`/`isize` are also enums with variants for 32 and 64 bit. At creation and various usage steps there are assertions in place checking if the target bitwidth matches with the chosen enum variant 5. enum discriminants (`ty::Disr`) are now `ConstInt` 6. trans has its own `Disr` type now (newtype around `u64`) This obviously can't be done without breaking changes (the ones that are noticable in stable) We could probably write lints that find those situations and error on it for a cycle or two. But then again, those situations are rare and really bugs imo anyway: ```rust let v10 = 10 as i8; let v4 = 4 as isize; assert_eq!(v10 << v4 as usize, 160 as i8); ``` stops compiling because 160 is not a valid i8 ```rust struct S<T, S> { a: T, b: u8, c: S } let s = S { a: 0xff_ff_ff_ffu32, b: 1, c: 0xaa_aa_aa_aa as i32 }; ``` stops compiling because `0xaa_aa_aa_aa` is not a valid i32 ---- cc @eddyb @pnkfelix related: rust-lang/rfcs#1071
@oli-obk did rust-lang/rust#30587 merging mean that we can close this, you think? |
After further review of rust-lang/rust#30587, I decided that yes, we can close this. |
Currently our handling of types in const-evaluation is pretty ad-hoc.
See for example: rust-lang/rust#23833 or more generally several on the list at rust-lang/rust#23897
The const-evaluator often tries to act like it can figure out an expected type from the context and pass it in, but this is not always possible in general.
And in any case, it may make more sense to have the const-evaluated values carry around the actual computed types. But then this leads to issues like: Should the computed types be so strict as to be "this is the value
-3
of typei8
"? Or would it be better to be more flexible and have the computed type just be "this is the value-3
of typesigned integer
, so that it could be used with any signed integer type that can hold that value?The text was updated successfully, but these errors were encountered: