]> git.lizzy.rs Git - rust.git/commit
Auto merge of #67016 - lqd:placeholder_loans, r=matthewjasper
authorbors <bors@rust-lang.org>
Mon, 9 Dec 2019 10:50:41 +0000 (10:50 +0000)
committerbors <bors@rust-lang.org>
Mon, 9 Dec 2019 10:50:41 +0000 (10:50 +0000)
commit3ff17e7c5faf604dcbfcb96a786df78e80f7e4f4
treeed56abf85c9d0c6a023326c2ff108f2e93252942
parentdbbe4f10fa68105223af6096df617ebb7ca59a48
parent1314ba323b6612d5109344c1d8bf9ae16e1e421f
Auto merge of #67016 - lqd:placeholder_loans, r=matthewjasper

In which we implement illegal subset relations errors using Polonius

This PR is the rustc side of implementing subset errors using Polonius. That is, in
```rust
fn foo<'a, 'b>(x: &'a u32, y: &'b u32) -> &'a u32 {
    y
}
```
returning `y` requires that `'b: 'a` but we have no evidence of that, so this is an error. (Evidence that the relation holds could come from explicit bounds, or via implied bounds).

Polonius outputs one such error per CFG point where the free region's placeholder loan unexpectedly flowed into another free region. While all these CFG locations could be useful in diagnostics in the future, rustc does not do that (and the duplication is only partially handled in the rest of the errors/diagnostics infrastructure, e.g. duplicate suggestions will be shown by the "outlives suggestions" or some of the `#[rustc_*]` NLL/MIR debug dumps), so I deduplicated the errors.

(The ordering also matters, otherwise some of the elided lifetime naming would change behaviour).

I've blessed a couple of tests, where the output is currently suboptimal:
- the `hrtb-perfect-forwarding` tests mix subset errors with higher-ranked subtyping, however the plan is for chalk to eventually take care of some of this to generate polonius constraints (i.e. it's not polonius' job). Until that happens, polonius will not see the error that NLL sees.
- some other tests have errors and diagnostics specific to `'static`, I _believe_ this to be because of it being treated as more "special" than in polonius. I believe the output is not wrong, but could be better, and appears elsewhere (I feel we'll need to look at polonius' handling of `'static` at some point in the future, maybe to match a bit more what NLL does when it produces errors)

I'll create a tracking issue in the polonius repo to record these 2 points (and a general "we'll need to go over the blessed output" issue, much like we did for NLLs)

The last blessed test is because it's an improvement: in this case, more errors/suggestions were computed, instead of the existing code path where this case apparently stops at the first error.

The `Naive` variant in Polonius computes those errors, so this PR also switches the default variant to that, as we're also in the process of temporarily deactivating all other variants (which exist mostly for performance considerations) until we have completed more work on completeness and correctness, before focusing on efficiency once again.

While most of the correctness in this PR is hidden in the polonius compare-mode (which of course passes locally), I've added a couple of smoke-tests to the existing ones, so that we have some confidence that it works (and keeps working) until we're in a position where we can run them on CI.

As mentioned during yesterday's wg-polonius meeting, @nikomatsakis has already read through most of this PR (and which is matching  what they thought needed to be done [during the recent Polonius sprint](https://hackmd.io/CGMNjt1hR_qYtsR9hgdGmw#Compiler-notes-on-generating-the-placeholder-loans-support)), but Matthew was hopefully going to review (again, not urgent), so:

r? @matthewjasper

(This updates to the latest `polonius-engine` release, and I'm not sure whether `Cargo.lock` updates can easily be rolled up, but apart from that: this changes little that's tested on CI, so seems safe-ish to rollup ?)
Cargo.lock
src/librustc/Cargo.toml
src/librustc_mir/borrow_check/nll/mod.rs
src/librustc_mir/borrow_check/nll/region_infer/mod.rs