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

Allow loop in constant evaluation #2344

Merged
merged 5 commits into from
Jul 2, 2018
Merged
Changes from 3 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
103 changes: 103 additions & 0 deletions text/0000-const-looping.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
- Feature Name: const_looping
- Start Date: 2018-02-18
- RFC PR: (leave this empty)
- Rust Issue: (leave this empty)

# Summary
[summary]: #summary

Allow the use of `loop`, `while` and `while let` during constant evaluation.
`for` loops are technically allowed, too, but can't be used in practice because
each iteration calls `iterator.next()`, which is not a `const fn` and thus can't
Copy link
Contributor

Choose a reason for hiding this comment

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

Linking #2237 which (or a modified version of which) will fix that and let you use iter.next() in const fn.

be called within constants. Future RFCs (like
https://github.com/rust-lang/rfcs/pull/2237) might lift that restriction.

# Motivation
[motivation]: #motivation

Any iteration is expressible with recursion. Since we already allow recursion
via const fn and termination of said recursion via `if` or `match`, all code
enabled by const recursion is already legal now. Some algorithms are better
expressed as imperative loops and a lot of Rust code uses loops instead of
recursion. Allowing loops in constants will allow more functions to become const
fn without requiring any changes.

# Guide-level explanation
[guide-level-explanation]: #guide-level-explanation

If you previously had to write functional code inside constants, you can now
change it to imperative code. For example if you wrote a fibonacci like
Copy link
Contributor

Choose a reason for hiding this comment

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

Who doesn't love a fibonacci example =P This section feels very nicely written!
Stylistically I'ld do: s/like/like: (and before the other sections as well..)


```rust
const fn fib(n: u128) -> u128 {
match n {
0 => 1,
1 => 1,
n => fib(n - 1) + fib(n + 1)
}
}
```

which takes exponential time to compute a fibonacci number, you could have
changed it to the functional loop

```rust
const fn fib(n: u128) -> u128 {
const fn helper(n: u128, a: u128, b: u128, i: u128) -> u128 {
if i <= n {
helper(n, b, a + b, i + 1)
} else {
b
}
}
helper(n, 1, 1, 2)
}
```

but now you can just write it as an imperative loop, which also finishes in
linear time.

```rust
const fn fib(n: u128) -> u128 {
let mut a = 1;
let mut b = 1;
let mut i = 2;
while i <= n {
let tmp = a + b;
a = b;
b = tmp;
i += 1;
}
b
}
```

# Reference-level explanation
[reference-level-explanation]: #reference-level-explanation

A loop in MIR is a cyclic graph of `BasicBlock`s. Evaluating such a loop is no
different from evaluating a linear sequence of `BasicBlock`s, except that
termination is not guaranteed. To ensure that the compiler never hangs
indefinitely, we count the number of terminators processed and whenever we reach
a fixed limit, we report a lint mentioning that we cannot guarantee that the
evaluation will terminate and reset the counter to zero. This lint should recur
in a non-annoying amount of time (e.g. at least 30 seconds between occurrences).
Copy link
Member

Choose a reason for hiding this comment

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

So just to be clear: the const-eval state will include both a terminator-counter and a timestamp; When the terminator-counter both 1. exceeds the (here unspecified) fixed limit, and 2. the current time delta from the timestamp is non-annoying, then we report the lint, reset the terminator-counter and timestamp, and finally resume const-eval execution?

(I'm just checking that there's not a typo here and that we are planning on using both pieces of state for the filter here, something of which I am in favor.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current implementation just has a MIR terminator counter for the warning. There's no host time check. Shouldn't be too hard to add.

Copy link
Member

Choose a reason for hiding this comment

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

I just want to clarify the end-intent. The current implementation isn't really something that concerns me all that much. :)


# Drawbacks
[drawbacks]: #drawbacks

* Infinite loops will hang the compiler if the lint is not denied
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Even just a #[warn] on the lint will cause warning output to occur (and repeat ad nauseaum) in response to an infinite loop, right? Usually when I hear "hang" I think of "no output visible"... so I would personally have written "if the lint is #[allow]'ed" here. But that might just be my own personal interpretation.


# Rationale and alternatives
[alternatives]: #alternatives

- Do nothing, users can keep using recursion

# Unresolved questions
[unresolved]: #unresolved-questions

* Should we add a true recursion check that hashes the interpreter state and
detects if it has reached the same state again?
* This will slow down const evaluation enormously and for complex iterations
is essentially useless because it'll take forever (e.g. counting from 0 to
`u64::max_value()`)