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

Towards dynamic const-qualify #27

Merged
merged 10 commits into from
Oct 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ which sort of is a virtual machine using `MIR` as "bytecode".
## Table of Contents

* [Const Safety](const_safety.md)
* [Constants](const.md)
* [Promotion](promotion.md)
* The three "kinds" of compile-time evaluated data:
* [Statics](static.md) (`static`, `static mut`)
* [Constants](const.md) (`const`, array sizes, non-`Copy` array initializers)
* [Promoteds](promotion.md) (rvalue promotion)

## Related RFCs

Expand Down
52 changes: 30 additions & 22 deletions const.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
# Further restrictions for constants
# Constants

The [const safety](const_safety.md) concerns apply to all computations happening
at compile-time, `static` and `const` alike. However, there are some additional
considerations about `const` specifically. These arise from the idea that
"Constants" in this document refers to `const` bodies, array sizes, and non-`Copy` array initializers.
On top of what applies to [statics](static.md), they are subject to an additional constraint: In code like
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
```rust
const CONST: T = EXPR;
```
is supposed to behave as-if `EXPR` was written at every use site of `CONST`.
To make this work, we need to ensure [const safety](const_safety.md).

Based on this requirement, we allow other constants and [promoteds](promotion.md) to read from constants.
This is why the value of a `const` is subject to validity checks.

## References

Expand All @@ -23,31 +26,42 @@ const REF: &u32 = { const _VAL = EXPR; static _STATIC = EXPR; &_STATIC };
(`EXPR` is assigned to a `const` first to make it subject to the restrictions
discussed in this document.)

There are three reasons why this could be an issue.
There are various reasons why this could be an issue.

### Pointer equality
### 1. Pointer equality

We effectively "deduplicate" all the allocations that would otherwise locally be
created at each use site of `REF`. This is observable when the program compares
these pointers for equality. We consider this okay, i.e., programs may not rely
on such constants all getting distinct addresses. They may not rely on them all
getting the same address either.

### Interior mutability
### 2. Interior mutability

If the reference has type `&Cell<i32>` it is quite clear that the program can
easily observe whether two references point to the same memory even without
comparing their address: Changes through one reference will affect reads through
the other. So, we cannot allow constant references to types that have interior
mutability (types that are not `Freeze`).
mutability (types that are not `Freeze`):

```rust
const BAD: &Cell<i32> = &Cell::new(42);
// Inlining `BAD` everywhere clearly is not the same as them all pointing to the same thing.
```

However, we can do better than that: Even if a *type* is not `Freeze`, it can
have *values* that do not exhibit any interior mutability. For example, `&None`
at type `&Option<Cell<i32>>` would be rejected by the naive analysis above, but
is actually accepted by the compiler because we know that there is no
`UnsafeCell` here that would permit interior mutability.

### `Sync`
*Dynamic check.* The Miri engine enforces this dynamically by ensuring that the
new data that is interned for a constant is all marked as immutable. However,
note the FIXME added [by this PR](https://github.com/rust-lang/rust/pull/63955):
for untyped data in a constant, we currently just *make* it immutable, instead
of checking properly.

### 3. `Sync`

Finally, the same constant reference is actually shared across threads. This is
very similar to multiple threads having a shared reference to the same `static`,
Expand All @@ -59,12 +73,11 @@ ecosystem that would break if we just started enforcing this now. See
[this issue](https://github.com/rust-lang/rust/issues/49206) and the
[PR attempting to fix this](https://github.com/rust-lang/rust/pull/54424/).

### `Drop`
*Dynamic check.* It is unclear how the Miri engine could dynamically check this.

Values of "needs drop" types
can only be used as the final initialization value of a `const` or `static` item.
They may not be used as intermediate values that would be dropped before the item
were initialized. As an example:
### 4. Drop

`Drop` is actually not an issue, at least not more so than for statics:

```rust
struct Foo;
Expand All @@ -76,19 +89,11 @@ impl Drop for Foo {
}

const FOO: Foo = Foo; // Ok, drop is run at each use site in runtime code
static FOOO: Foo = Foo; // Ok, drop is never run

// Not ok, cannot run `Foo::drop` because it's not a const fn
const BAR: i32 = (Foo, 42).1;
```

This restriction might be lifted in the future after trait impls
may be declared `const` (https://github.com/rust-rfcs/const-eval/pull/8).

Note that in promoteds this restriction can never be lifted, because
otherwise we would silently stop calling the `Drop` impl at runtime and
pull it to much earlier (compile-time).

## Reading statics

Beyond values of reference type, we have to be careful that *computing* a
Expand All @@ -101,3 +106,6 @@ This is distinct to the concern about interior mutability above: That concern
was about first computing a `&Cell<i32>` and then using it at run-time (and
observing the fact that it has been "deduplicated"), this here is about using
such a value at compile-time even though it might be changed at run-time.

*Dynamic check.* The Miri engine could check this dynamically by refusing to
access mutable global memory when computing a const.
95 changes: 77 additions & 18 deletions promotion.md
Original file line number Diff line number Diff line change
@@ -1,25 +1,32 @@
# Const promotion

"Promotion" is a mechanism that affects code like `&3`: Instead of putting it on
the stack, the `3` is allocated in global static memory and a reference with
lifetime `'static` is provided. This is essentially an automatic transformation
turning `&EXPR` into `{ const _PROMOTED = &EXPR; EXPR }`, but only if `EXPR`
qualifies.
["(Implicit) Promotion"][promotion-rfc] is a mechanism that affects code like `&3`:
Instead of putting it on the stack, the `3` is allocated in global static memory
and a reference with lifetime `'static` is provided. This is essentially an
automatic transformation turning `&EXPR` into
`{ const _PROMOTED = &EXPR; EXPR}`, but only if `EXPR` qualifies.

Note that promotion happens on the MIR, not on surface-level syntax. This is
relevant when discussing e.g. handling of panics caused by overflowing
arithmetic.

On top of what applies to [consts](const.md), promoteds suffer from the additional issue that *the user did not ask for them to be evaluated at compile-time*.
Thus, if CTFE fails but the code would have worked fine at run-time, we broke the user's code for no good reason.
Even if we are sure we found an error in the user's code, we are only allowed to [emit a warning, not a hard error][warn-rfc].
That's why we have to be very conservative with what can and cannot be promoted.

[promotion-rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1414-rvalue_static_promotion.md
[warn-rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1229-compile-time-asserts.md

## Rules

### 1. Panics

Promotion is not allowed to throw away side effects. This includes panicking.
Let us look at what happens when we promote `&(0_usize - 1)` in a debug build:
We have to avoid erroring at compile-time, because that would be promotion
breaking compilation (the code would have compiled just fine if we hadn't
promoted), but we must be sure to error correctly at run-time. In the MIR, this
looks roughly like
breaking compilation, but we must be sure to error correctly at run-time. In
the MIR, this looks roughly like

```
_tmp1 = CheckedSub (const 0usize) (const 1usize)
Expand All @@ -46,6 +53,9 @@ earlier version of miri used to panic on arithmetic overflow even in release
mode. This breaks promotion, because now promoting code that would work (and
could not panic!) at run-time leads to a compile-time CTFE error.

*Dynamic check.* The Miri engine already dynamically detects panics, but the
main point of promoteds is ruling them out statically.

### 2. Const safety

We have explained what happens when evaluating a promoted panics, but what about
Expand All @@ -62,7 +72,7 @@ everything, so the only possible remaining failure are panics.

However, things get more tricky when `const` and `const fn` are involved.

For `const`, based on the const safety check described [here](const_safety.md),
For `const`, based on the const safety check described [here](const_safety.md#const-safety-check-on-values),
we can rely on there not being const-unsafe values in the `const`, so we should
be able to promote freely. For example:

Expand All @@ -89,18 +99,18 @@ but to abort compilation of a program that would have compiled fine if we would
not have decided to promote. It is the responsibility of `foo` to not fail this
way when working with const-safe arguments.

### 3. Constraints on constants
For this reason, only `const fn` that were explicitly marked with the
`#[rustc_promotable]` attribute are subject to promotion.

All the [extra restrictions for constants](const.md) beyond const safety also
apply to promoteds, for the same reason: Evaluating the expression at
compile-time instead of run-time should not alter program behavior.
*Dynamic check.* The Miri engine already dynamically detects const safety
violations, but the main point of promoteds is ruling them out statically.

### 4. Drop
### 3. Drop

Expressions containing "needs drop" types
can never be promoted. If such an expression were promoted, the `Drop` impl would
never get called on the value, even though the user did not explicitly request such
behavior by using an explicit `const` or `static` item.
Expressions returning "needs drop" types can never be promoted. If such an
expression were promoted, the `Drop` impl would never get called on the value,
even though the user did not explicitly request such behavior by using an
explicit `const` or `static` item.

As expression promotion is essentially the silent insertion of a `static` item, and
`static` items never have their `Drop` impl called, the `Drop` impl of the promoted
Expand All @@ -111,6 +121,55 @@ it is unlikely to be the desired behavior in most cases and very likey to be con
to the user. If such behavior is desired, the user can still use an explicit `static`
or `const` item and refer to that.

*Dynamic check.* The Miri engine could dynamically check this by ensuring that
the result of computing a promoted is a value that does not need dropping.

## `&` in `const` and `static`

Promotion is also responsible for making code like this work:

```rust
const FOO: &'static i32 = {
let x = &13;
x
};
```

However, since this is in explicit const context, we are less strict about
promotion in this situation: all function calls are promoted, not just
`#[rustc_promotable]` functions:

```rust
const fn bar() -> i32 { 42 }

const FOO: &'static i32 = {
let x = &bar(); // this gets promoted
x
};
```

However, we still do not promote *everything*; e.g., drop-checking still applies:

```rust
const DROP: &'static Vec<u8> = {
let x = &Vec::new(); //~ ERROR: temporary value dropped while borrowed
x
};
```

Notice that some code involving `&` *looks* like it relies on promotion but
actually does not:

```rust
const EMPTY_BYTES: &Vec<u8> = &Vec::new(); // Ok without promotion
```

As we have seen above, `Vec::new()` does not get promoted. And yet this
compiles. Why that? The reason is that the reference obtains the lifetime of
the "enclosing scope", similar to how `let x = &mut x;` creates a reference
whose lifetime lasts for the enclosing scope. This is decided during MIR
building already, and does not involve promotion.

## Open questions

* There is a fourth kind of CTFE failure -- resource exhaustion. What do we do
Expand Down
36 changes: 36 additions & 0 deletions static.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Statics

Statics (`static`, `static mut`) are the simplest kind of compile-time evaluated data:
* The user explicitly requested them to be evaluated at compile-time,
so evaluation errors from computing the initial value of a static are no concern
(in other words, [const safety](const_safety.md) is mostly not an issue).
* They observably get evaluated *once*, with the result being put at some address known at run-time,
so there are no fundamental restrictions on what statics can do.
* The compiler checks that statics are `Sync`, justifying sharing their address across threads.
* [Constants](const.md) and [promoteds](promotion.md) are not allowed to read from statics,
so their final value does not have have to be [const-valid](const_safety.md#const-safety-check-on-values) in any meaningful way.
As of 2019-08, we do check them for validity anyway, to be conservative; and indeed constants could be allowed to read from frozen statics.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be nice personally; less times to invent an ephemeral constant just to please the check and you also get to use exported statics from other crates.

Any future compat hazards with some new feature?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discovered a problem... If we want to have a scheme like rust-lang/rfcs#2492 but centered around statics instead, allowing statics in const contexts will give us all the problems re. separate compilation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Centril sorry, I do not follow... why is it harder for separate compilation to support const referring to static than const referring to const (and the latter we already support)?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case it would be a static for which the value isn't known yet; that is something like pub extern static Foo: u8; <-- no value. That would be similar to rust-lang/rfcs#2492 in that it is matched up with something in the crate graph. If you can then refer to Foo in types then that will create all the same problems as with extern existential type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as whatever instantiates the static checks them for validity, we are sill good. The important part is the invariant that all statics have been checked. The order doesn't matter.


## `Drop`

The compiler rejects intermediate values (created and discarded during the computation of a static initializer) that implement `Drop`.
The reason for this is simply that the `Drop` implementation might be non-`const fn`.
This restriction can be lifted once `const impl Drop for Type` (or something similar) is supported.

```rust
struct Foo;

impl Drop for Foo {
fn drop(&mut self) {
println!("foo dropped");
}
}

static FOOO: Foo = Foo; // Ok, drop is never run

// Not ok, cannot run `Foo::drop` because it's not a const fn
static BAR: i32 = (Foo, 42).1;
```

*Dynamic check.* The Miri engine dynamically checks that this is done correctly
by not permitting calls of non-`const` functions.