Skip to content
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

Closed
pnkfelix opened this issue Apr 18, 2015 · 9 comments
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.

Comments

@pnkfelix
Copy link
Member

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 type i8"? Or would it be better to be more flexible and have the computed type just be "this is the value -3 of type signed integer, so that it could be used with any signed integer type that can hold that value?

@pnkfelix
Copy link
Member Author

(I consider rust-lang/rust#15270 to be a specific instance of this, so I am not planning to clone that issue.)

@pnkfelix
Copy link
Member Author

potentially related recent PR attempt: rust-lang/rust#18538

@ghost
Copy link

ghost commented Apr 19, 2015

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.

@pnkfelix
Copy link
Member Author

"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 ;)

@oli-obk
Copy link
Contributor

oli-obk commented May 13, 2015

in case it's dynamically typed

  • deprecate type annotations on constants
  • there'll be two different systems, constant and runtime, with totally different behavior and syntax
    • lots of confused programmers
    • lots of magic
    • basically a new language for compile time computation :(

in case it's statically typed

"iff" may be a bit stronger than I would like.

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.

@pnkfelix
Copy link
Member Author

@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"

@oli-obk
Copy link
Contributor

oli-obk commented Dec 21, 2015

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).

bors added a commit to rust-lang/rust that referenced this issue Mar 14, 2016
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
@nrc nrc added the T-lang Relevant to the language team, which will review and decide on the RFC. label Aug 30, 2016
critiqjo pushed a commit to critiqjo/rustdoc that referenced this issue Dec 16, 2016
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
@pnkfelix
Copy link
Member Author

pnkfelix commented Jan 8, 2018

@oli-obk did rust-lang/rust#30587 merging mean that we can close this, you think?

@pnkfelix
Copy link
Member Author

pnkfelix commented Jan 8, 2018

After further review of rust-lang/rust#30587, I decided that yes, we can close this.

@pnkfelix pnkfelix closed this as completed Jan 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

3 participants