]> git.lizzy.rs Git - rust.git/commit - src/tools/clippy
Auto merge of #68528 - ecstatic-morse:maybe-init-variants, r=oli-obk
authorbors <bors@rust-lang.org>
Thu, 27 Feb 2020 15:17:47 +0000 (15:17 +0000)
committerbors <bors@rust-lang.org>
Thu, 27 Feb 2020 15:17:47 +0000 (15:17 +0000)
commit49c68bd53f90e375bfb3cbba8c1c67a9e0adb9c0
tree61f805498957efbd2eae99b877c8a6eb3baa7a81
parenta8437cf213ac1e950b6f5c691c4d2a29bf949bcd
parente2c80477532b54271cbb7249d0bb36d7b257fca2
Auto merge of #68528 - ecstatic-morse:maybe-init-variants, r=oli-obk

Mark other variants as uninitialized after switch on discriminant

During drop elaboration, which builds the drop ladder that handles destruction during stack unwinding, we attempt to remove MIR `Drop` terminators that will never be reached in practice. This reduces the number of basic blocks that are passed to LLVM, which should improve performance. In #66753, a user pointed out that unreachable `Drop` terminators are common in functions like `Option::unwrap`, which move out of an `enum`. While discussing possible remedies for that issue, @eddyb suggested moving const-checking after drop elaboration. This would allow the former, which looks for `Drop` terminators and replicates a small amount of drop elaboration to determine whether a dropped local has been moved out, leverage the work done by the latter.

However, it turns out that drop elaboration is not as precise as it could be when it comes to eliminating useless drop terminators. For example, let's look at the code for `unwrap_or`.

```rust
fn unwrap_or<T>(opt: Option<T>, default: T) -> T {
    match opt {
        Some(inner) => inner,
        None => default,
    }
}
```

`opt` never needs to be dropped, since it is either moved out of (if it is `Some`) or has no drop glue (if it is `None`), and `default` only needs to be dropped if `opt` is `Some`. This is not reflected in the MIR we currently pass to codegen.

![pasted_image](https://user-images.githubusercontent.com/29463364/73384403-109a0d80-4280-11ea-8500-0637b368f2dc.png)

@eddyb also suggested the solution to this problem. When we switch on an enum discriminant, we should be marking all fields in other variants as definitely uninitialized. I implemented this on top of alongside a small optimization (split out into #68943) that suppresses drop terminators for enum variants with no fields (e.g. `Option::None`). This is the resulting MIR for `unwrap_or`.

![after](https://user-images.githubusercontent.com/29463364/73384823-e432c100-4280-11ea-84bd-d0bcc3b777b4.png)

In concert with #68943, this change speeds up many [optimized and debug builds](https://perf.rust-lang.org/compare.html?start=d55f3e9f1da631c636b54a7c22c1caccbe4bf0db&end=0077a7aa11ebc2462851676f9f464d5221b17d6a). We need to carefully investigate whether I have introduced any miscompilations before merging this. Code that never drops anything would be very fast indeed until memory is exhausted.
src/librustc/mir/mod.rs
src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs
src/librustc_mir/borrow_check/mod.rs
src/librustc_mir/borrow_check/nll.rs
src/librustc_mir/dataflow/generic/engine.rs
src/librustc_mir/dataflow/generic/mod.rs
src/librustc_mir/dataflow/impls/mod.rs