-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
1ace1e4
turn statically known erroneous code into a warning and an unconditio…
oli-obk 1a7110b
add a definition of constant evaluation context
oli-obk 3855520
minor clarifications
oli-obk 8107908
add const fn unresolved question
oli-obk 1354de7
added c-like enums and patterns to const context list
oli-obk 61d1436
use alternative "warn + normal codegen" instead
oli-obk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
- 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. | ||
|
||
# Definition of constant evaluation context | ||
|
||
There are exactly three places where an expression needs to be constant. | ||
|
||
- the initializer of a constant `const foo: ty = EXPR` or `static foo: ty = EXPR` | ||
- the size of an array `[T; EXPR]` | ||
- the length of a repeat expression `[VAL; LEN_EXPR]` | ||
|
||
In the future the body of `const fn` might also be interpreted as a constant | ||
evaluation context. | ||
|
||
Any other expression might still be constant evaluated, but it could just | ||
as well be compiled normally and executed at runtime. | ||
|
||
# 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 while evaluating | ||
an expression outside of a constant evaluation environment, 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 | ||
|
||
The const evaluator should simply issue a warning and notify it's caller that the expression cannot be evaluated and should be translated. | ||
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. | ||
|
||
On the other hand, this alternative does not change the behavior of existing code. | ||
|
||
# Unresolved questions | ||
|
||
## How to implement this? | ||
|
||
## Const-eval the body of `const fn` that are never used in a constant environment | ||
|
||
Currently a `const fn` that is called in non-const code is treated just like a normal function. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (brief discussion leads me to think that the context of a body of a |
||
|
||
In case there is a statically known erroneous situation in the body of the function, | ||
the compiler should raise an error, even if the function is never called. | ||
|
||
The same applies to unused associated constants. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a complete list... off the top of my head, patterns and enum discriminants are also required to be constant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed