typestrong const integers #30587

oli-obk · 2015-12-28T13:27:59Z

~~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:

~~const eval array values when const evaluating an array expression~~
~~const eval repeat value when const evaluating a repeat expression~~
~~const eval all struct and tuple fields when evaluating a struct/tuple expression~~
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
enum discriminants (ty::Disr) are now ConstInt
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:

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

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
#13768
#23833

rust-highfive · 2015-12-28T13:28:13Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Aatch (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

bors · 2015-12-28T17:27:34Z

☔ The latest upstream changes (presumably #30588) made this pull request unmergeable. Please resolve the merge conflicts.

nagisa · 2015-12-28T22:07:40Z

src/doc/reference.md

@@ -3677,10 +3677,10 @@ sites are:

 * `let` statements where an explicit type is given.

-   For example, `128` is coerced to have type `i8` in the following:


Nice catch, but probably should go into a separate rollable-up PR.

bors · 2015-12-31T19:00:49Z

☔ The latest upstream changes (presumably #30660) made this pull request unmergeable. Please resolve the merge conflicts.

nagisa · 2016-01-06T00:52:07Z

This will have to be reviewed outside-of-github because of its tendency to censor.

bors · 2016-01-11T17:19:49Z

☔ The latest upstream changes (presumably #30676) made this pull request unmergeable. Please resolve the merge conflicts.

oli-obk · 2016-01-13T14:08:56Z

I removed the eager const eval to do that on its own. For now it's just the typestrong const integers.

I'm not sure what the expected behavior of something like 160 as i8 is, but if it's "random value" instead of an error, then I can easily introduce that by turning it into 42 deterministically outside of true constants (and emitting a warning oc).

brson · 2016-01-13T18:24:02Z

I'll run crater.

brson · 2016-01-14T19:12:56Z

Report. Only regression is a false positive.

oli-obk · 2016-01-14T19:24:55Z

wooohoo xD I'll split it up into multiple PRs for easier reviewing

bors · 2016-01-15T21:39:09Z

☔ The latest upstream changes (presumably #30890) made this pull request unmergeable. Please resolve the merge conflicts.

This is groundwork for #30587 (typestrong constant integrals), but imo it's a change that in itself is good, too, since we don't just juggle `u64`s around anymore. `ty::Disr` will be changed to a `ConstInt` in #30587

`assert_eq!(-9223372036854775808isize as u64, 0x8000000000000000);` fails on 32 bit and succeeds on 64 bit. These commits don't change that behavior. The following worked before my changes, because the discriminant was always processed as `u64`. Now it fails, because the discriminant of `Bu64` is now `0` instead of `-9223372036854775808`. This is more in line with the above assertion's code, since `-9223372036854775808isize as u64` on 32 bit yielded `0`. ```rust enum Eu64 { Au64 = 0, Bu64 = 0x8000_0000_0000_0000 } ```

oli-obk · 2016-03-11T09:53:48Z

As the commit message of fcee002 states:

assert_eq!(-9223372036854775808isize as u64, 0x8000000000000000);

fails on 32 bit and succeeds on 64 bit. These commits don't change that behavior.

The following worked before my changes, because the discriminant was
always processed as u64.
Now it fails, because on 32 bit the discriminant of Bu64 is now 0 instead of -9223372036854775808. This is more in line with the above assertion's code, since
-9223372036854775808isize as u64 on 32 bit yielded 0.

enum Eu64 {
    Au64 = 0,
    Bu64 = 0x8000_0000_0000_0000
}

nikomatsakis · 2016-03-11T22:36:36Z

@oli-obk thanks; can't say I'm entirely happy with the rules here, but this seems like the best step for now.

nikomatsakis · 2016-03-11T22:37:31Z

@bors r+

bors · 2016-03-11T22:37:32Z

📌 Commit 08c448a has been approved by nikomatsakis

bors · 2016-03-11T22:53:16Z

⌛ Testing commit 08c448a with merge f5dbfb4...

bors · 2016-03-11T23:25:02Z

💔 Test failed - auto-linux-64-opt-rustbuild

arielb1 · 2016-03-13T12:20:12Z

Try to add rustc_const_eval to the rustbuild config. cc @alexcrichton

alexcrichton · 2016-03-13T20:21:25Z

Yeah if this is adding a new crate it'll just need a Cargo.toml plus the necessary dependency edges to be made as well.

oli-obk · 2016-03-14T08:51:09Z

can't test locally, because the boostrap crate's build script doesn't know about --enable-clang

but I added the dependencies to every crate that has extern crate rustc_const_eval

alexcrichton · 2016-03-14T16:42:52Z

@bors: r=nikomatsakis f665c39

bors · 2016-03-14T18:38:24Z

⌛ Testing commit f665c39 with merge 0111892...

@eddyb

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

bors · 2016-03-14T21:44:52Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-ios-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

oli-obk · 2016-03-14T21:47:28Z

🎉 🎈

rust-highfive assigned Aatch Dec 28, 2015

oli-obk force-pushed the eager_const_eval2 branch from e2e8e47 to 0354271 Compare December 28, 2015 15:21

oli-obk force-pushed the eager_const_eval2 branch from e03966c to 50245af Compare December 28, 2015 19:02

nagisa reviewed Dec 28, 2015
View reviewed changes

pnkfelix added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Dec 29, 2015

oli-obk force-pushed the eager_const_eval2 branch from 50245af to e24a1de Compare January 11, 2016 15:11

oli-obk force-pushed the eager_const_eval2 branch from 7b9bc2e to b4f78a1 Compare January 13, 2016 13:49

oli-obk changed the title ~~WIP: eager const evaluation + typestrong const integers~~ WIP: typestrong const integers Jan 13, 2016

brson removed the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Jan 14, 2016

oli-obk mentioned this pull request Jan 15, 2016

use a newtype for the variant discriminant in trans instead of u64 #30931

Merged

oli-obk force-pushed the eager_const_eval2 branch from b4f78a1 to 50245af Compare January 16, 2016 12:48

oli-obk added 2 commits March 10, 2016 12:50

fix rustdoc

7003ed3

don't be a breaking change, even in presence of overflowing literals

d4d6260

oli-obk force-pushed the eager_const_eval2 branch from 6b61528 to b6b192b Compare March 10, 2016 11:50

oli-obk force-pushed the eager_const_eval2 branch from b6b192b to fcee002 Compare March 10, 2016 13:21

undo changes to run-pass tests that aren't errors anymore

08c448a

oli-obk force-pushed the eager_const_eval2 branch from cb7b0a1 to 08c448a Compare March 11, 2016 08:44

rustbuild

f665c39

bors merged commit f665c39 into rust-lang:master Mar 14, 2016

bors mentioned this pull request Mar 14, 2016

Restrict constants in patterns #32199

Merged

oli-obk deleted the eager_const_eval2 branch March 14, 2016 21:47

This was referenced Mar 14, 2016

Implement RFC 1210: impl specialization #30652

Merged

derive: assume enum repr defaults to isize #32253

Merged

This was referenced Jan 8, 2018

Should const-evaluation be dynamically-typed or statically-typed? (And with what types?) rust-lang/rfcs#1071

Closed

metabug: const_eval bugs from type confusion and/or overflow #23897

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

typestrong const integers #30587

typestrong const integers #30587

oli-obk commented Dec 28, 2015

rust-highfive commented Dec 28, 2015

bors commented Dec 28, 2015

nagisa Dec 28, 2015

bors commented Dec 31, 2015

nagisa commented Jan 6, 2016

bors commented Jan 11, 2016

oli-obk commented Jan 13, 2016

brson commented Jan 13, 2016

brson commented Jan 14, 2016

oli-obk commented Jan 14, 2016

bors commented Jan 15, 2016

oli-obk commented Mar 11, 2016

nikomatsakis commented Mar 11, 2016

nikomatsakis commented Mar 11, 2016

bors commented Mar 11, 2016

bors commented Mar 11, 2016

bors commented Mar 11, 2016

arielb1 commented Mar 13, 2016

alexcrichton commented Mar 13, 2016

oli-obk commented Mar 14, 2016

alexcrichton commented Mar 14, 2016

bors commented Mar 14, 2016

bors commented Mar 14, 2016

oli-obk commented Mar 14, 2016

		@@ -3677,10 +3677,10 @@ sites are:

		* `let` statements where an explicit type is given.

		For example, `128` is coerced to have type `i8` in the following:

typestrong const integers #30587

typestrong const integers #30587

Conversation

oli-obk commented Dec 28, 2015

rust-highfive commented Dec 28, 2015

bors commented Dec 28, 2015

nagisa Dec 28, 2015

Choose a reason for hiding this comment

bors commented Dec 31, 2015

nagisa commented Jan 6, 2016

bors commented Jan 11, 2016

oli-obk commented Jan 13, 2016

brson commented Jan 13, 2016

brson commented Jan 14, 2016

oli-obk commented Jan 14, 2016

bors commented Jan 15, 2016

oli-obk commented Mar 11, 2016

nikomatsakis commented Mar 11, 2016

nikomatsakis commented Mar 11, 2016

bors commented Mar 11, 2016

bors commented Mar 11, 2016

bors commented Mar 11, 2016

arielb1 commented Mar 13, 2016

alexcrichton commented Mar 13, 2016

oli-obk commented Mar 14, 2016

alexcrichton commented Mar 14, 2016

bors commented Mar 14, 2016

bors commented Mar 14, 2016

oli-obk commented Mar 14, 2016