[MIR] broken MIR (bad assignment) for non-returning closure

[alexcrichton]

This code:

fn main() {
    let o: Option<u32> = None;

    let foo = || -> ! {
        loop {}
    };

    o.unwrap_or_else(|| {
        foo();
    });
}

Will yield this warning:

main.rs:8:25: 10:6 warning: broken MIR (return = ()): bad assignment (u32 = ()): Sorts(ExpectedFound { expected: u32, found: () })
main.rs: 8     o.unwrap_or_else(|| {
main.rs: 9         foo();
main.rs:10     });

[nagisa]

This happens because we construct MIR for the 2nd closure as if foo closure was converging. That’s trivially wrong. I’m looking into fixing this.

[nagisa]

Okay, so:

1. You cannot have ! as an associated type;
2. Fn has Self::Output as return type for closures;
3. MIR uses <[[email protected]:4:15: 6:6] as core::ops::Fn<()>>::call(tmp1, tmp3) to call the offending closure in question.

---

Specifically for MIR, we could stop using Fn::call and have some special case for closures, but to me it seems like the issue is deeper rooted than MIR here.

[arielb1]

Can we have diverging closures, anyway?

[arielb1]

The closure has the output type (), not !. This code should not compile (as splitting it into separate functions should demonstrate). Or we should support diverging closures which requires !.

[arielb1]

This seems like a serious backwards-compatibility hazard. We seem to have forced ourselves to introduce a ! type. Unless we call this code "just a bug".

[nikomatsakis]

Hmm. I thought that || -> ! was disallowed now.

[nikomatsakis]

That is, we are not supposed to have diverging closures!

[canndrew]

We seem to have forced ourselves to introduce a ! type.

You say that like it's a bad thing.

That is, we are not supposed to have diverging closures!

Well that's just a crappy, arbitrary restriction that we can get rid of. Also we do have diverging closures because you can write || -> Void

[arielb1]

@canndrew

I don't want to have MIR blocked on new RFCs, basically. Maybe we can get that done fast enough.

Well that's just a crappy, arbitrary restriction that we can get rid of. Also we do have diverging closures because you can write || -> Void

A || -> Void closure is not treated as diverging for dataflow purposes - you have to match on it for that.

[nikomatsakis]

On Sat, Apr 16, 2016 at 02:33:55AM -0700, Andrew Cann wrote:

That is, we are not supposed to have diverging closures!

Well that's just a crappy, arbitrary restriction that we can get rid of. Also we do have diverging closures because you can write || -> Void

I don't really disagree. I'd just prefer to make that decision on its
own merits, not because of this oversight. But oh well: the failure to
completely remove -> ! closures (which is my oversight as much as
anyone's) does kind of force our hand. We have to fix it one way or
another, and it's certainly worth thinking if we can fix it in a
backwards-compatible way (e.g., by making ! into a type, and
assigning stronger semantics to empty enums, or at least to Void).

[nikomatsakis]

Discussed in @rust-lang/lang team meeting -- we think it makes sense to reconsider the RFC that makes ! into a sort of type.

We also should try to find some sort of way to "unregress" this test case in the short term, but I'm not sure what's the best plan there.

[nagisa]

The code sample does not emit the warning anymore, but please keep the issue open because it was hacked around only.

[nagisa]

Now that we have actual never type, it might be interesting to revisit this.