Stream: t-compiler/wg-nll

Topic: conditional-control-flow-across-functions-and-iterators


Jake Goulding (Jun 22 2018 at 22:01, on Zulip):

I found a case where the boring, non-iterator based code works with NLL+Polonius, but the idiomatic iterator version does not:

// rustc 1.28.0-nightly (ae46aefd5 2018-06-16)
// cargo +nightly rustc -- -Z polonius
#![feature(nll)]

struct Reader;
struct Data;
struct Errors<'a>(&'a str);

fn handle_array(stream: &mut Reader) -> Result<Data, Errors> {
    // Works
    let mut array = vec![];
    for _ in (0..0) {
        array.push(handle_request(stream)?);
    }

    // Does not work
    let array = (0..0)
        .map(|_| handle_request(stream))
        .collect::<Result<Vec<_>, _>>()?;

    unimplemented!();
}

fn handle_request(_stream: &mut Reader) -> Result<Data, Errors> {
    unimplemented!()
}

fn main() {}

Is this worth an issue?

lqd (Jun 22 2018 at 22:09, on Zulip):

I would think so :)

Jake Goulding (Jun 22 2018 at 22:09, on Zulip):

Oh, I guess that can't be safe. Since it could be collected into a Vec<Errors> which would then allow for mutable aliasing, yeah?

Vytautas Astrauskas (Jun 25 2018 at 13:27, on Zulip):

@Jake Goulding For your example, the compiler reports what I would expect it to report. The type of the first array is Vec<Data> – therefore, we know that it does not capture the stream. However, the type of the second array is Vec<Result<Data, Errors<'a>>>, which may capture thestream. I guess that if you managed to write something similar to map(|_| handle_request(stream)?), then also the second version would compile.

Jake Goulding (Jun 25 2018 at 13:29, on Zulip):

that sounds like it agrees with my followup comment; do you concur?

Vytautas Astrauskas (Jun 25 2018 at 13:43, on Zulip):

I mostly agree with your follow up comment. To me, it seems that both versions are actually safe because Errors take a shared reference. However, since the compiler does the modular analysis, it does not see that and to be on the safe side reports an error. In other words: the compiler thinks that there could be mutable aliasing and, therefore, rejects the code.

Jake Goulding (Jun 25 2018 at 13:49, on Zulip):

Errors take a shared reference.

Well, but even if that's the case, something would be holding the immutable reference while the loop continues on trying to use the mutable reference.

I agree it ultimately seems safe, but that's only because we "know" that the implementation of <Vec as FromIterator> / Vec::push doesn't look at the references it holds as it goes on to the next element

Vytautas Astrauskas (Jun 25 2018 at 16:44, on Zulip):

Yes, you are completely right. I forgot about the iterator.

nikomatsakis (Jun 25 2018 at 19:45, on Zulip):

@Jake Goulding a nit. I would've found this so much easier to read if you would use '_:

fn handle_request(_stream: &mut Reader) -> Result<Data, Errors<'_>> {
    unimplemented!()
}

As is, I spent a long time being puzzled because I didn't realize that Errors carried a lifetime

Jake Goulding (Jun 25 2018 at 19:46, on Zulip):

@Jake Goulding a nit. I would've found this so much easier to read if you would use '_:

Hey, make it stable and I'll start getting used to it ;-)

nikomatsakis (Jun 25 2018 at 19:47, on Zulip):

I think it is

nikomatsakis (Jun 25 2018 at 19:47, on Zulip):

but maybe not?

nikomatsakis (Jun 25 2018 at 19:47, on Zulip):

that said, the lint that encourages you to migrate I think still needs work

Jake Goulding (Jun 25 2018 at 19:49, on Zulip):

I think it is

Well, I'll be.

struct A<'a>(&'a ());

struct X;
impl X {
    fn x(&self) -> A<'_> {
        unimplemented!()
    }
}

fn main() {}
Jake Goulding (Jun 25 2018 at 19:49, on Zulip):

And yeah, there doesn't seem to be a lint...

Jake Goulding (Jun 25 2018 at 19:49, on Zulip):

But yeah, that's cool it's stable. Wonder when that happened.

nikomatsakis (Jun 25 2018 at 19:50, on Zulip):

there is a lint

nikomatsakis (Jun 25 2018 at 19:50, on Zulip):

it's just not enabled by default

Jake Goulding (Jun 25 2018 at 19:51, on Zulip):

it's just not enabled by default

Dangerously close to "if a tree falls in the forest" territory here. :wink:

Jake Goulding (Jun 25 2018 at 19:56, on Zulip):

Ok, It was only stabilized in 1.26, so that's not too long for me to have been missing out

Last update: Nov 21 2019 at 13:10UTC