Implement RFC 458: improve Send

[huonw]

This removes the 'static bound from Send, and implements Send for &'a T ( T: Sync) and &'a mut T (T: Send), as per rust-lang/rfcs#458, and so closes #22251.

This is rebased on top of Niko's default object lifetime work.

---

This makes a parallel for function like

fn par_for<I, F>(iter: I, f: F)
    where I: Iterator,
          <I as Iterator>::Item: Send,
          F: Fn(<I as Iterator>::Item) + Sync
{
    let _guards: Vec<_> = iter.map(|elem| {
        Thread::scoped(|| {
            f(elem)
        })
    }).collect();

}

compile, run in parallel, and work with essentially arbitrary iterators, including over references of arrays stored on the stack etc.

[rust-highfive]

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

[huonw]

r? @nikomatsakis

---

This isn't quite ready for merging, since this compiles, but it seems like it shouldn't, since y is a pointer into the stack frame of the bad closure, but the join guard is being used outside bad, it seems the lifetime restriction in Thread::scoped should stop the join guard lasting longer than the closure itself.

#![feature(core, std_misc)]
use std::thread::Thread;


fn main() {
    let bad = || {
        let x = 1;
        let y = &x;

        Thread::scoped(|| {
            let z = y;
            1 + *z
        })
    };

    println!("{}", bad().join().ok().unwrap());
}

[huonw]

This also does essentially no auditing, just mindlessly adding an additional 'static bound to most instances of Send in std, except in std::thread and std::thunk.

cc @pythonesque, @aturon.

[pythonesque]

I take it that after auditing, the expectation is that we'll be able to remove 'static from a lot of the other types as well?

[alexcrichton]

Could this actually continue to have 'static on std::thread in light of #20807?

[nikomatsakis]

Exciting! I'll take a look, and in particular take a look at the unsafety errors.

[nikomatsakis]

@alexcrichton

Could this actually continue to have 'static on std::thread in light of #20807?

I personally think that'd be rather disappointing. We'd be giving up quite a lot of expressiveness (and gaining that expressiveness was sort of the point of the changes to Send/Sync).

[huonw]

Re-adding 'static until #20807 is resolved doesn't lose anything compared to our current libs...

Then again, I do want my safe parallel_map right now! :P

[pythonesque]

@huonw Looking at your example, I'm not fully convinced it's unsafe. Doesn't the closure last until bad itself goes out of scope? I don't think it has at the point where you use the join guard. I could easily be confused, though (_edit:_ yep, pretty sure I was just confused :P)

[nikomatsakis]

@huonw that's true, it doesn't lose anything relative to now. I'm just not happy with it as a permanent solution (though I am fine if we wind up with a different API).

Anyway, the problem you encountered is, I think, is the fact that 'a in JoinGuard is not used anywhere. This was fixed in the variance PR (since an unconstrained lifetime is a hard error there). But in the meantime adding this patch makes your example work fine:

lunch-box. git diff libstd/
diff --git a/src/libstd/thread.rs b/src/libstd/thread.rs
index 3b758c8..f291e83 100644
--- a/src/libstd/thread.rs
+++ b/src/libstd/thread.rs
@@ -255,6 +255,7 @@ impl Builder {
             joined: false,
             packet: my_packet,
             thread: thread,
+            _marker: [],
         }
     }

@@ -487,6 +488,7 @@ pub struct JoinGuard<'a, T: 'a> {
     thread: Thread,
     joined: bool,
     packet: Packet<T>,
+    _marker: [&'a T; 0], // bind the lifetime `'a`
 }

 #[stable(feature = "rust1", since = "1.0.0")]

(As an aside I recently realized that [T;0] is a "poor man's" phantom data, in that it has precisely the same effect.)

[nikomatsakis]

Also this is a simpler example that exhibited the same problem (no need for closures):

#![feature(core, std_misc)]
use std::thread::Thread;

fn main() {
    let bad = {
        let x = 1;
        let y = &x;

        Thread::scoped(|| {
            let z = y;
            1 + *z
        })
    };

    println!("{}", bad.join().ok().unwrap());
}

[nikomatsakis]

I added a few comments of places I do not think the 'static bound is needed -- basically wrappers around types, like Mutex, Future, and channels should be able to do without. But it may be worth loosening those in a separate PR, as I imagine we'd want to write some run-pass and compile-fail tests checking that the type system is enforcing that you don't leak references.

[alexcrichton]

@alexcrichton

Could this actually continue to have 'static on std::thread in light of #20807?

I personally think that'd be rather disappointing. We'd be giving up quite a lot of expressiveness (and gaining that expressiveness was sort of the point of the changes to Send/Sync).

Oh sorry I should have mentioned that I, too, would love to remove the 'static bound. I was just thinking in terms of "we may tweak the API slightly" but it's #[unstable] anyway so it's fine either way.

[nikomatsakis]

well, modulo the need for a marker on JoinGuard, the PR looks good to me. I think it makes sense to leave FIXMEs for the 'static bounds I identified as being ones we could loosen, and then come back and make sure we have pos/neg tests for them.

[nikomatsakis]

(I'm somewhat indifferent on whether we take that same approach for JoinGuard or not.)

[japaric]

Shouldn't _marker prefer PhantomData<&'a T> over [&'a T; 0]?

Anyway, I'm excited to get sendable references/slices 😍.

[huonw]

Updated, tests pass.

[alexcrichton]

@bors: r+ 7a14f49