[MIR] Implement overflow checking
The initial set of changes is from @Aatch's #33255 PR, rebased on master, plus:
Added an `Assert` terminator to MIR, to simplify working with overflow and bounds checks.
With this terminator, error cases can be accounted for directly, instead of looking for lang item calls.
It also keeps the MIR slimmer, with no extra explicit blocks for the actual panic calls.
Warnings can be produced when the `Assert` is known to always panic at runtime, e.g.:
```rust
warning: index out of bounds: the len is 1 but the index is 3
--> <anon>:1:14
1 |> fn main() { &[std::io::stdout()][3]; }
|> ^^^^^^^^^^^^^^^^^^^^^^
```
Generalized the `OperandValue::FatPtr` optimization to any aggregate pair of immediates.
This allows us to generate the same IR for overflow checks as old trans, not something worse.
For example, addition on `i16` calls `llvm.sadd.with.overflow.i16`, which returns `{i16, i1}`.
However, the Rust type `(i16, bool)`, has to be `{i16, i8}`, only an immediate `bool` is `i1`.
But if we split the pair into an `i16` and an `i1`, we can pass them around as such for free.
The latest addition is a rebase of #34054, updated to work for pairs too. Closes#34054, fixes#33873.
Last but not least, the `#[rustc_inherit_overflow_checks]` attribute was introduced to control the
overflow checking behavior of generic or `#[inline]` functions, when translated in another crate.
It is **not** intended to be used by crates other than `libcore`, which is in the unusual position of
being distributed as only an optimized build with no checks, even when used from debug mode.
Before MIR-based translation, this worked out fine, as the decision for overflow was made at
translation time, in the crate being compiled, but MIR stored in `rlib` has to contain the checks.
To avoid always generating the checks and slowing everything down, a decision was made to
use an attribute in the few spots of `libcore` that need it (see #33255 for previous discussion):
* `core::ops::{Add, Sub, Mul, Neg, Shl, Shr}` implementations for integers, which have `#[inline]` methods and can be used in generic abstractions from other crates
* `core::ops::{Add, Sub, Mul, Neg, Shl, Shr}Assign` same as above, for augmented assignment
* `pow` and `abs` methods on integers, which intentionally piggy-back on built-in multiplication and negation, respectively, to get overflow checks
* `core::iter::{Iterator, Chain, Peek}::count` and `core::iter::Enumerate::{next, nth}`, also documented as panicking on overflow, from addition, counting elements of an iterator in an `usize`
generate fewer basic blocks for variant switches
CC #33567
Adds a new field to TestKind::Switch that tracks the variants that are actually matched against. The other candidates target a common "otherwise" block.
this introduces a DropAndReplace terminator as a fix to #30380. That terminator
is suppsoed to be translated by desugaring during drop elaboration, which is
not implemented in this commit, so this breaks `-Z orbit` temporarily.
Fixes to mir dataflow
Fixes to mir dataflow
This collects a bunch of changes to `rustc_borrowck::borrowck::dataflow` (which others have pointed out should probably migrate to some crate that isn't tied to the borrow-checker -- but I have not attempted that here, especially since there are competing approaches to dataflow that we should also evaluate).
These changes:
1. Provide a family of related analyses: MovingOutStatements (which is what the old AST-based dataflo computed), as well as MaybeInitialized, MaybeUninitalized, and DefinitelyInitialized.
* (The last two are actually inverses of each other; we should pick one and drop the other.)
2. Fix bugs in the pre-existing analysis implementation, which was untested and thus some obvious bugs went unnoticed, which brings us to the third point:
3. Add a unit test infrastructure for the MIR dataflow analysis.
* The tests work by adding a new intrinsic that is able to query the analysis state for a particular expression (technically, a particular L-value).
* See the examples in compile-fail/mir-dataflow/inits-1.rs and compile-fail/mir-dataflow/uninits-1.rs
* These tests are only checking the results for MaybeInitialized, MaybeUninitalized, and DefinitelyInitialized; I am not sure if it will be feasible to generalize this testing strategy to the MovingOutStatements dataflow operator.
(The crucial thing these changes are working toward (but are not yet
in this commit) is a way to pretty-print MIR without having the
`NodeId` for that MIR in hand.)
Some simple improvements to MIR pretty printing
In short, this PR changes the MIR printer so that it:
* places an empty line between the MIR for each item
* does *not* write an empty line before the first BB when there are no
var decls
* aligns the "// Scope" comments 50 chars in (makes the output more
readable)
* prints the scope comments as "// scope N at ..." instead of "//
Scope(N) at ..."
* prints a prettier scope tree:
* no more unbalanced delimiters!
* no more "Parent" entry (these convey no useful information)
* drop the "Scope()" and just print scope IDs
* no braces when the scope is empty
In action: https://gist.github.com/jonas-schievink/1c11226cbb112892a9470ce0f9870b65
Only break critical edges where actually needed
Currently, to prepare for MIR trans, we break _all_ critical edges,
although we only actually need to do this for edges originating from a
call that gets translated to an invoke instruction in LLVM.
This has the unfortunate effect of undoing a bunch of the things that
SimplifyCfg has done. A particularly bad case arises when you have a
C-like enum with N variants and a derived PartialEq implementation.
In that case, the match on the (&lhs, &rhs) tuple gets translated into
nested matches with N arms each and a basic block each, resulting in N²
basic blocks. SimplifyCfg reduces that to roughly 2*N basic blocks, but
breaking the critical edges means that we go back to N².
In nickel.rs, there is such an enum with roughly N=800. So we get about
640K basic blocks or 2.5M lines of LLVM IR. LLVM takes a while to
reduce that to the final "disr_a == disr_b".
So before this patch, we had 2.5M lines of IR with 640K basic blocks,
which took about about 3.6s in LLVM to get optimized and translated.
After this patch, we get about 650K lines with about 1.6K basic blocks
and spent a little less than 0.2s in LLVM.
cc #33111
r? @Aatch
mir: don't attempt to promote Unpromotable constant temps.
Fixes#33537. This was a non-problem in regular functions, but we also promote in `const fn`s.
There we always qualify temps so you can't depend on `Unpromotable` temps being `NOT_CONST`.
In short, this PR changes the MIR printer so that it:
* places an empty line between the MIR for each item
* does *not* write an empty line before the first BB when there are no
var decls
* aligns the "// Scope" comments 50 chars in (makes the output more
readable)
* prints the scope comments as "// scope N at ..." instead of "//
Scope(N) at ..."
* prints a prettier scope tree:
* no more unbalanced delimiters!
* no more "Parent" entry (these convey no useful information)
* drop the "Scope()" and just print scope IDs
* no braces when the scope is empty
Currently, to prepare for MIR trans, we break _all_ critical edges,
although we only actually need to do this for edges originating from a
call that gets translated to an invoke instruction in LLVM.
This has the unfortunate effect of undoing a bunch of the things that
SimplifyCfg has done. A particularly bad case arises when you have a
C-like enum with N variants and a derived PartialEq implementation.
In that case, the match on the (&lhs, &rhs) tuple gets translated into
nested matches with N arms each and a basic block each, resulting in N²
basic blocks. SimplifyCfg reduces that to roughly 2*N basic blocks, but
breaking the critical edges means that we go back to N².
In nickel.rs, there is such an enum with roughly N=800. So we get about
640K basic blocks or 2.5M lines of LLVM IR. LLVM takes a while to
reduce that to the final "disr_a == disr_b".
So before this patch, we had 2.5M lines of IR with 640K basic blocks,
which took about about 3.6s in LLVM to get optimized and translated.
After this patch, we get about 650K lines with about 1.6K basic blocks
and spent a little less than 0.2s in LLVM.
cc #33111
mir: drop temps outside-in by scheduling the drops inside-out.
It was backwards all along, but only noticeable with multiple drops in one rvalue scope. Fixes#32433.
Coercion casts (`expr as T` where the type of `expr` can be coerced to
`T`) are essentially no-ops, as the actual work is done by a coercion.
Previously a check for type equality was used to avoid emitting the
redundant cast in the MIR, but this failed for coercion casts of
function items that had lifetime parameters. The MIR trans code doesn't
handle `FnPtr -> FnPtr` casts and produced an error.
Also fixes a bug with type ascription expressions not having any
adjustments applied.
Fixes#33295
Primarily affects the MIR construction, which indirectly improves LLVM
IR generation, but some LLVM IR changes have been made too.
* Handle "statement expressions" more intelligently. These are
expressions that always evaluate to `()`. Previously a temporary would
be generated as a destination to translate into, which is unnecessary.
This affects assignment, augmented assignment, `return`, `break` and
`continue`.
* Avoid inserting drops for non-drop types in more places. Scheduled
drops were already skipped for types that we knew wouldn't need
dropping at construction time. However manually-inserted drops like
those for `x` in `x = y;` were still generated. `build_drop` now takes
a type parameter like its `schedule_drop` counterpart and checks to
see if the type needs dropping.
* Avoid generating an extra temporary for an assignment where the types
involved don't need dropping. Previously an expression like
`a = b + 1;` would result in a temporary for `b + 1`. This is so the
RHS can be evaluated, then the LHS evaluated and dropped and have
everything work correctly. However, this isn't necessary if the `LHS`
doesn't need a drop, as we can just overwrite the existing value.
* Improves lvalue analysis to allow treating an `Rvalue::Use` as an
operand in certain conditions. The reason for it never being an
operand is so it can be zeroed/drop-filled, but this is only true for
types that need dropping.
The first two changes result in significantly fewer MIR blocks being
generated, as previously almost every statement would end up generating
a new block due to the drop of the `()` temporary being generated.
Paths are mostly parsed without taking whitespaces into account, e.g. `std :: vec :: Vec :: new ()` parses successfully, however, there are some special cases involving keywords `super`, `self` and `Self`. For example, `self::` is considered a path start only if there are no spaces between `self` and `::`. These restrictions probably made sense when `self` and friends weren't keywords, but now they are unnecessary.
The first two commits remove this special treatment of whitespaces by removing `token::IdentStyle` entirely and therefore fix https://github.com/rust-lang/rust/issues/14109.
This change also affects naked `self` and `super` (which are not tightly followed by `::`, obviously) they can now be parsed as paths, however they are still not resolved correctly in imports (cc @jseyfried, see `compile-fail/use-keyword.rs`), so https://github.com/rust-lang/rust/issues/29036 is not completely fixed.
The third commit also makes `super`, `self`, `Self` and `static` keywords nominally (before this they acted as keywords for all purposes) and removes most of remaining \"special idents\".
The last commit (before tests) contains some small improvements - some qualified paths with type parameters are parsed correctly, `parse_path` is not used for parsing single identifiers, imports are sanity checked for absence of type parameters - such type parameters can be generated by syntax extensions or by macros when https://github.com/rust-lang/rust/issues/10415 is fixed (~~soon!~~already!).
This patch changes some pretty basic things in `libsyntax`, like `token::Token` and the keyword list, so it's a plugin-[breaking-change].
r? @eddyb
MIR: Do not require END_BLOCK to always exist
Basically, all this does, is removing restriction for END_BLOCK to exist past the first invocation of RemoveDeadBlocks pass. This way for functions whose CFG does not reach the `END_BLOCK` end up not containing the block.
As far as the implementation goes, I’m not entirely satisfied with the `BasicBlock::end_block`. I had hoped to make `new` a `const fn` and then just have a `const END_BLOCK` private to mir::build, but it turns out that constant functions don’t yet support conditionals nor a way to assert.
Once upon a time, along with START_BLOCK and END_BLOCK in the castle of important blocks also lived
a RESUME_BLOCK (or was it UNWIND_BLOCK? Either works, I don’t remember anymore). This trinity of
important blocks were required to always exist from the birth to death of the MIR-land they
belonged to.
Some time later, it was discovered that RESUME_BLOCK was just a lazy goon enjoying comfortable life
in the light of fame of the other two. Needless to say, once found out, the RESUME_BLOCK was
quickly slain and disposed of.
Now, the all-seeing eye of ours discovers that END_BLOCK is actually the more evil and better
disguised twin of the slain RESUME_BLOCK. Thus END_BLOCK gets slain and quickly disposed
of. Glory to the START_BLOCK, one and only lord of the important blocks’ castle!
---
Basically, all this does, is removing restriction for END_BLOCK to exist past the first invocation
of RemoveDeadBlocks pass. This way for functions whose CFG does not reach the `END_BLOCK` end up
not containing the block.
As far as the implementation goes, I’m not entirely satisfied with the `BasicBlock::end_block`, I
had hoped to make `new` a `const fn` and then just have a `const END_BLOCK` private to mir::build,
but it turns out that constant functions don’t yet support conditionals nor a way to assert.
Plumb obligations through librustc/infer
Like #32542, but more like #31867.
TODO before merge: make an issue for the propagation of obligations through... uh, everywhere... then replace the `#????`s with the actual issue number.
cc @jroesch
r? @nikomatsakis
rBreak Critical Edges and other MIR work
This PR is built on top of #32080.
This adds the basic depth-first traversals for MIR, preorder, postorder and reverse postorder. The MIR blocks are now translated using reverse postorder. There is also a transform for breaking critical edges, which includes the edges from `invoke`d calls (`Drop` and `Call`), to account for the fact that we can't add code after an `invoke`. It also stops generating the intermediate block (since the transform essentially does it if necessary already).
The kinds of cases this deals with are difficult to produce, so the test is the one I managed to get. However, it seems to bootstrap with `-Z orbit`, which it didn't before my changes.
Also adds a new set of passes to run just before translation that
"prepare" the MIR for codegen. Removal of landing pads, region erasure
and break critical edges are run in this pass.
Also fixes some merge/rebase errors.
This is a fairly standard transform that inserts blocks along critical
edges so code can be inserted along the edge without it affecting other
edges. The main difference is that it considers a Drop or Call
terminator that would require an `invoke` instruction in LLVM a critical
edge. This is because we can't actually insert code after an invoke, so
it ends up looking similar to a critical edge anyway.
The transform is run just before translation right now.
Restrict constants in patterns
This implements [RFC 1445](https://github.com/rust-lang/rfcs/blob/master/text/1445-restrict-constants-in-patterns.md). The primary change is to limit the types of constants used in patterns to those that *derive* `Eq` (note that implementing `Eq` is not sufficient). This has two main effects:
1. Floating point constants are linted, and will eventually be disallowed. This is because floating point constants do not implement `Eq` but only `PartialEq`. This check replaces the existing special case code that aimed to detect the use of `NaN`.
2. Structs and enums must derive `Eq` to be usable within a match.
This is a [breaking-change]: if you encounter a problem, you are most likely using a constant in an expression where the type of the constant is some struct that does not currently implement
`Eq`. Something like the following:
```rust
struct SomeType { ... }
const SOME_CONST: SomeType = ...;
match foo {
SOME_CONST => ...
}
```
The easiest and most future compatible fix is to annotate the type in question with `#[derive(Eq)]` (note that merely *implementing* `Eq` is not enough, it must be *derived*):
```rust
struct SomeType { ... }
const SOME_CONST: SomeType = ...;
match foo {
SOME_CONST => ...
}
```
Another good option is to rewrite the match arm to use an `if` condition (this is also particularly good for floating point types, which implement `PartialEq` but not `Eq`):
```rust
match foo {
c if c == SOME_CONST => ...
}
```
Finally, a third alternative is to tag the type with `#[structural_match]`; but this is not recommended, as the attribute is never expected to be stabilized. Please see RFC #1445 for more details.
cc https://github.com/rust-lang/rust/issues/31434
r? @pnkfelix
This is a [breaking-change]: according to RFC #1445, constants used as
patterns must be of a type that *derives* `Eq`. If you encounter a
problem, you are most likely using a constant in an expression where the
type of the constant is some struct that does not currently implement
`Eq`. Something like the following:
```rust
struct SomeType { ... }
const SOME_CONST: SomeType = ...;
match foo {
SOME_CONST => ...
}
```
The easiest and most future compatible fix is to annotate the type in
question with `#[derive(Eq)]` (note that merely *implementing* `Eq` is
not enough, it must be *derived*):
```rust
struct SomeType { ... }
const SOME_CONST: SomeType = ...;
match foo {
SOME_CONST => ...
}
```
Another good option is to rewrite the match arm to use an `if`
condition (this is also particularly good for floating point types,
which implement `PartialEq` but not `Eq`):
```rust
match foo {
c if c == SOME_CONST => ...
}
```
Finally, a third alternative is to tag the type with
`#[structural_match]`; but this is not recommended, as the attribute is
never expected to be stabilized. Please see RFC #1445 for more details.
while I'm at it, remove the "extra caching" that I was doing for no good
reason except laziness. Basically before I was caching at each scope in
the chain, but there's not really a reason to do that, since the cached
entry point at level N is always equal to the last cached exit point
from level N-1.
It's nice to be able to index with a scope-id,
but coherence rules prevent us from implementing
`Index<ScopeId>` for `Vec<ScopeAuxiliary>`, and I'd
prefer that `ScopeAuxiliary` remain in librustc_mir,
just for compilation time reasons.
This was triggered by me wanting to address a use of DUMMY_SP, but
actually I'm not sure what would be a better span -- I guess the span
for the function as a whole.
Automated conversion using the untry tool [1] and the following command:
```
$ find -name '*.rs' -type f | xargs untry
```
at the root of the Rust repo.
[1]: https://github.com/japaric/untry
emit (via debug!) scary message from `fn borrowck_mir` until basic
prototype is in place.
Gather children of move paths and set their kill bits in
dataflow. (Each node has a link to the child that is first among its
siblings.)
Hooked in libgraphviz based rendering, including of borrowck dataflow
state.
doing this well required some refactoring of the code, so I cleaned it
up more generally (adding comments to explain what its trying to do
and how it is doing it).
Update: this newer version addresses most review comments (at least
the ones that were largely mechanical changes), but I left the more
interesting revisions to separate followup commits (in this same PR).
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: https://github.com/rust-lang/rfcs/issues/1071
Add Pass manager for MIR
A new PR, since rebasing the original one (https://github.com/rust-lang/rust/pull/31448) properly was a pain. Since then there has been several changes most notable of which:
1. Removed the pretty-printing with `#[rustc_mir(graphviz/pretty)]`, mostly because we now have `--unpretty=mir`, IMHO that’s the direction we should expand this functionality into;
2. Reverted the infercx change done for typeck, because typeck can make an infercx for itself by being a `MirMapPass`
r? @nikomatsakis
There's a lot of stuff wrong with the representation of these types:
TyFnDef doesn't actually uniquely identify a function, TyFnPtr is used to
represent method calls, TyFnDef in the sub-expression of a cast isn't
correctly reified, and probably some other stuff I haven't discovered yet.
Splitting them seems like the right first step, though.
