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

turn statically known erroneous code into a warning and continue normal code-generation #1229

Merged
merged 6 commits into from
Sep 4, 2015
Merged
Changes from 1 commit
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
88 changes: 88 additions & 0 deletions text/0000-compile-time-asserts.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
- Feature Name: compile_time_asserts
- Start Date: 2015-07-30
- RFC PR: (leave this empty)
- Rust Issue: (leave this empty)

# Summary

If the compiler can detect at compile-time that something will always
cause a `debug_assert` or an `assert` it should instead
insert an unconditional runtime-panic and issue a warning.

# Motivation

Expressions are const-evaluated even when they are not in a const environment.

For example

```rust
fn blub<T>(t: T) -> T { t }
let x = 5 << blub(42);
```

will not cause a compiler error currently, while `5 << 42` will.
If the constant evaluator gets smart enough, it will be able to const evaluate
the `blub` function. This would be a breaking change, since the code would not
compile anymore. (this occurred in https://github.com/rust-lang/rust/pull/26848).

GNAT (an Ada compiler) does this already:

```ada
procedure Hello is
Var: Integer range 15 .. 20 := 21;
begin
null;
end Hello;
```

The anonymous subtype `Integer range 15 .. 20` only accepts values in `[15, 20]`.
This knowledge is used by GNAT to emit the following warning during compilation:

```
warning: value not in range of subtype of "Standard.Integer" defined at line 2
warning: "Constraint_Error" will be raised at run time
```

I don't have a GNAT with `-emit-llvm` handy, but here's the asm with `-O0`:

```asm
.cfi_startproc
pushq %rbp
.cfi_def_cfa_offset 16
.cfi_offset 6, -16
movq %rsp, %rbp
.cfi_def_cfa_register 6
movl $2, %esi
movl $.LC0, %edi
movl $0, %eax
call __gnat_rcheck_CE_Range_Check
```


# Detailed design

The PRs https://github.com/rust-lang/rust/pull/26848 and https://github.com/rust-lang/rust/pull/25570 will be setting a precedent
for warning about such situations (WIP, not pushed yet).
All future additions to the const-evaluator need to notify the const evaluator
that when it encounters a statically known erroneous situation, the
entire expression must be replaced by a panic and a warning must be emitted.

# Drawbacks

None, if we don't do anything, the const evaluator cannot get much smarter.

# Alternatives

## allow breaking changes

Let the compiler error on things that will unconditionally panic at runtime.

## only warn, don't influence code generation

This has the disadvantage, that in release-mode statically known issues like
overflow or shifting more than the number of bits available will not be
caught even at runtime.

# Unresolved questions

How to implement this?
Copy link
Member

Choose a reason for hiding this comment

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

Well, one easy way to implement it is: If you detect the error situation, then emit the warning, and then abandon the constant folding for that expression (that is, continue to emit the code that will run the expression at runtime, thus hitting the error condition when/if it runs).

Right?

Update: Oh, I didn't read carefully enough -- my strategy above (which I think is already included in the Alteratives section) differs from that of the RFC in that the above strategy will only work for -C debug-assertions mode builds, not release builds

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you also need to detect that you are not in a true const environment, where you definitely want this to happen and there can't be breakage since the feature wasn't there before