regression 1.50: Borrow checker reports function param as not living long enough

[rylev]

This might not actually be a regression an instead the code may have previously erroneously compiled. I cannot find an an open issue obviously related to this though.

As you can see here this code does not compile with a "borrowed value does not live long enough" where it did previously. This comes from a call to diff::diff which has this signature.

The error message states that n may be dropped while the borrow in the next param state is still active. This fails to compile on both 1.50 and 1.49 if the param state is move to a local variable.

@rustbot modify labels: +regression-from-stable-to-beta

[apiraino]

Let's try to reduce it @rustbot ping cleanup

[rustbot]

Hey Cleanup Crew ICE-breakers! This bug has been identified as a good
"Cleanup ICE-breaking candidate". In case it's useful, here are some
instructions for tackling these sorts of bugs. Maybe take a look?
Thanks! <3

cc @AminArria @camelid @chrissimpkins @contrun @DutchGhost @elshize @ethanboxx @h-michael @HallerPatrick @hdhoang @hellow554 @imtsuki @JamesPatrickGill @kanru @KarlK90 @LeSeulArtichaut @MAdrianMattocks @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @shekohex @sinato @smmalis37 @steffahn @Stupremee @tamuhey @turboladen @woshilapin @yerke

[hellow554]

Still with dependency, but simplified code

use euca::diff;
use euca::dom::Dom;
use euca::vdom::DomIter;

#[test]
fn from_empty() {
    let new: Dom<(), (), &()> = Dom::elem("div");

    let n = new.dom_iter();
    let mut storage = vec![];
    let patch_set = diff::diff(std::iter::empty(), n, &mut storage);
}

let's see what I can get from there

[hellow554]

searched nightlies: from nightly-2020-12-01 to nightly-2021-01-06
regressed nightly: nightly-2020-12-06
searched commits: from 3ff10e7 to e792288
regressed commit: 9122b76

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc 2021-01-06 --without-cargo --access github --regress error

The code I tested:

struct Dom;
impl Dom {
    fn dom_iter(&self) -> Box<dyn Iterator<Item = &()>> {
        todo!()
    }
}

fn diff<'a, M, O, N, S>(_: O, _: N, _: S)
where
    M: 'a,
    O: IntoIterator<Item = M>,
    N: IntoIterator<Item = &'a M>,
    S: IntoIterator<Item = &'a M>,
{
    todo!()
}

fn main() {
    let n = Dom.dom_iter();
    let storage = vec![];

    diff(std::iter::empty(), n, &storage);
}

cc #78373 @matthewjasper

[rustbot]

Error: Label ICEBreaker-Cleanup-Crew can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

[matthewjasper]

So #78373 resulted in MIR having one fewer block, which is making borrowck be less precise with how long the borrow lasts. There are a few possible fixes here for me to look at.

[apiraino]

Assigning P-high as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

[Mark-Simulacrum]

@rylev how common was this bug in the crater report? just the one crate?

[rylev]

Yes the linked crate was the only crate to show signs of this issue in the crater run.

[matthewjasper]

@nikomatsakis @pnkfelix @nagisa wanted a longer explanation of what's going on here.

When we type check the MIR we find that:

• The lifetime of the borrow at bb3[3] must outlive the lifetime in the type of _8
• the lifetimes in the types of _8 and _7 are the same because of the signature of diff
• The lifetime in the type of _7 is the same as the lifetime in the type of _1 (from the assignment at bb3[1])

So the lifetime of the borrow contains all points where _8, _7 and _1 are live and initialized, which I have marked in green. The borrow therefore lasts only for bb3[3] and bb3[4] (borrows only start at the statement with the borrow rvalue).

In the new MIR the drop of _7 is gone, so there's no longer a gap in the lifetime between bb3 and the drop of _4. This results in the borrow of _4 including bb7 and bb8, which conflicts with the drop of _4 in bb7.

[pnkfelix]

discussed at T-compiler meeting. We want to resolve this issue, and plan to revert PR #78373 as a short-term resolution. Longer term will be to re-land PR #78373 with hopefully some further modifications so that we don't cause regressions like this in the process.

[pnkfelix]

While setting up my system for the revert, I narrowed the MCVE down a bit further. I don't think I illuminated much more in the process, though I do find it curious that I wasn't able to easily find a way to take the Box<dyn Trait> out of the equation for reproducing the bug:

playground

trait Trait { type Item; }

impl<'a, X> Trait for &'a Vec<X> {
    type Item = &'a X;
}

impl<X> Trait for Box<dyn Trait<Item = X>> {
    type Item = X;
}

fn make_dyn_trait(_: &()) -> Box<dyn Trait<Item = &()>> {
    todo!()
}

fn diff<'a, M, N, S>(_: N, _: S)
where
    M: 'a,
    N: Trait<Item = &'a M>,
    S: Trait<Item = &'a M>,
{
    todo!()
}

fn may_panic<X>(_: X) { }

fn main() {
    let dyn_trait = make_dyn_trait(&());
    let storage = vec![()];
    let _x = may_panic(());
    let storage_ref = &storage;
    diff(dyn_trait, storage_ref);
}

[pnkfelix]

(landing PR #81257 and backporting it to the beta channel would resolve this.)

[mzabaluev]

Here is a minified example of a regression I have run into, not sure if it's the same issue:

use std::error::Error;
use std::iter;

fn walk_source_chain(error: &impl Error) {
    for e in iter::successors(error.source(), |e| e.source()) {
        println!("{}", e);
    }
}

Changing the closure parameter pattern to match &e fixes the borrow checker error in nightly.

[camelid]

Hmm, that example compiles on beta though.

[camelid]

I believe that's an unrelated issue. Opened #81460 to track it. Thanks for reporting it!

[pietroalbini]

Fix backported to 1.50 and landed in 1.51, closing this.