Removed all unneeded explicit lifetimes

[melbyruarus]

Explicit lifetimes make it impossible for each end of a channel to have differing lifespans, even though internally channels support this.

For example the following code snippet for a timer would not compile without this modification:

use comm::spsc::one_space;
use std::thread;

pub fn oneshot<'a>(ms: u32) -> one_space::Consumer<'a, ()> {
    let (tx, rx) = one_space::new();
    thread::spawn(move || {
        thread::sleep_ms(ms);
        tx.send(());
    });
    rx
}

This would fail with the following message:

src/timer.rs:5:17: 5:33 error: cannot infer an appropriate lifetime due to conflicting requirements
src/timer.rs:5  let (tx, rx) = one_space::new();
                               ^~~~~~~~~~~~~~~~
src/timer.rs:5:2: 5:34 note: first, the lifetime cannot outlive the destruction scope surrounding statement at 5:1...
src/timer.rs:5  let (tx, rx) = one_space::new();
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/timer.rs:5:17: 5:33 note: ...so that references are valid when the destructor runs
src/timer.rs:5  let (tx, rx) = one_space::new();
                               ^~~~~~~~~~~~~~~~
src/timer.rs:4:60: 11:2 note: but, the lifetime must be valid for the lifetime 'a as defined on the block at 4:59...
src/timer.rs:4 pub fn oneshot<'a>(ms: u32) -> one_space::Consumer<'a, ()> {
src/timer.rs:5  let (tx, rx) = one_space::new();
src/timer.rs:6     thread::spawn(move || {
src/timer.rs:7         thread::sleep_ms(ms);
src/timer.rs:8         tx.send(());
src/timer.rs:9     });
               ...
src/timer.rs:10:5: 10:7 note: ...so that expression is assignable (expected `comm::spsc::one_space::Consumer<'a, ()>`, found `comm::spsc::one_space::Consumer<'_, ()>`)
src/timer.rs:10     rx

The solution proposed in this pull request is to remove all explicit lifetimes, and instead lets the compiler infer these.

This change does break existing consumers of the API, both through the removal of lifetime specifiers, and by changing the interface of as_trait.

as_trait now takes a TraitObject instead of a Trait, and callers of this function need to change code of the form:

unsafe { self.data.as_trait(&*self.data as &(_Selectable+'a)) }

to:

unsafe { self.data.as_trait(ptr::read(&(&*self.data as &(_Selectable)) as *const _ as *const TraitObject)) }

[mahkoh]

This seems to compile:

extern crate comm;

use comm::spsc::one_space;
use std::thread;

pub fn oneshot(ms: u32) -> one_space::Consumer<'static, ()> {
    let (tx, rx) = one_space::new();
    thread::spawn(move || {
        thread::sleep_ms(ms);
        tx.send(());
    });
    rx
}

fn main() {
    oneshot(1000).recv_sync();
}

[melbyruarus]

I've now had time to have a more thorough look at lifetimes in rust and they now make a lot more sense. I can now see how this change isn't needed in the context I initially proposed.

I will still leave this PR open however, as the change it introduces does in my opinion make the API cleaner.

Is there a reason I am not aware of for which it is desirable to have explicit lifetime specifiers?

[mahkoh]

I assume you're aware that it's possible to send types with non-static lifetimes over channels. For example, you can send references to stack variables between scoped threads.

This alone does not require extra lifetimes, however, this library has a powerful Select structure which allows us to select on channels without borrowing them. The select structure holds references to _Selectable trait objects. These _Selectable objects (in our case, channels) have to hold references to the Select structures so that the select structures can be notified when a new message arrives on the channel. These references appear in the form of

    wait_queue:       UnsafeCell<WaitQueue<'a>>,

in the channels. These WaitQueue objects need a lifetime which is the lifetime of the _Selectable objects that can be placed in the Select structure. Since this lifetime has to be named, the channels themselves must have a named lifetime in their definitions.

What we would like to do is the following:

pub struct Packet<T: Sendable> {
    // ...
    wait_queue:       UnsafeCell<WaitQueue<lifetime_of(T)>>,
}

so that the lifetime does not have to be named. But this syntax does not exist so we have to name the lifetime.

In your patch you made the following change in the Select implementation:

-struct Entry<'a> {
-    data: WeakTrait<_Selectable<'a>+'a>,
+struct Entry {
+    data: WeakTrait<_Selectable>,
 }

https://github.com/mahkoh/comm/pull/20/files#diff-b267f2d25b5ff8596719813301a83e14R210

This is equivalent to

+struct Entry {
+    data: WeakTrait<_Selectable+'static>,
 }

so it only allows us to select on channels which send data with a static lifetime.

Tl;dr: The compiler is not powerful enough for us to omit the lifetimes.