feat: Add tagged notifications

[notgull]

Supersedes #40 by adding tags to the event. Similarly to #40, this PR:

• Adds a T parameter to Event and EventListener that is () by default.
• Breaking: Makes it so wait() and friends return T or Option<T> instead of () and bool.
• Makes it so polling EventListener as a future returns T instead of ().

Unlike #40, I wanted to use a more sustainable method for notifying the event. Introducing the Notification trait. Its API looks like this:

trait Notification {
    type Tag;
    fn is_additional(&self) -> bool;
    fn count(&self) -> usize;
    fn fence(&self);
    fn next_tag(&mut self) -> T;
}

This roughly corresponds to the implicit parameters to the current event.notify_<is_additional>_<relaxed>(count) function. is_additional sets the additional property to true, and relaxed makes the fence do nothing instead of emitting a SeqCst fence. The next_tag function emits the tag that gets attached to the notification.

I wanted to give this some power, so I created the IntoNotification trait as well.

trait IntoNotification {
    type Tag;
    type Notify: Notification<Tag = Self::Tag>;
    fn into_notification(self) -> Self::Notify;
    
    fn additional(self) -> Additional<Self::Notify> { .. }
    fn relaxed(self) -> Relaxed<Self::Notify> { .. }
    fn tag<T: Clone>(self, tag: T) -> Tag<Self::Notify, T> { .. }
    fn tag_with<T, F: FnMut() -> T>(self, f: F) -> TagWith<Self::Notify, F> { .. }
}

impl<N: Notification> IntoNotification for N { .. }
impl IntoNotification for usize { .. }

This way, if we make notify() take an impl IntoNotification instead of usize, it still works as intended, as the implementation for usize::into_notification is non-additional and non-relaxed:

event.notify(3);

You can also use this system to add combinators (.additional(), .relaxed()), such that:

// Old code:
event.notify_additional(3);

// New code:
event.notify(3.additional());

Then, to add tags to a notification, you would do:

// Assume tag is a Vec<usize>
event.notify(3.tag(vec![1, 2, 3]));

// Alternate method
event.notify(3.tag_with(|| vec![1, 2, 3]));

Discussion questions:

• Should Notification and IntoNotification be sealed? If it is, it means we can add new methods to it without worrying. If it isn't, it means that users can implement their own notification types. However, that may cause behavior we don't expect.

[notgull]

Blocked on taiki-e/portable-atomic#93, since portable_atomic_util::Arc doesn't implement Unpin and thus can't be projected until it does. Fixed!

[fogti]

Should Notification and IntoNotification be sealed? If it's not, it means we can add new methods to it without worrying. If it is, it means that users can implement their own notification types. However, that may cause behavior we don't expect.

I think you've messed up the negation of "sealed" here.

Also, if we don't seal it, it might be a good idea to split up the IntoNotification trait up, so that the functions which users should be able to customize are separated from the rest (where customization wouldn't make sense).

[notgull]

I think you've messed up the negation of "sealed" here.

Also, if we don't seal it, it might be a good idea to split up the IntoNotification trait up, so that the functions which users should be able to customize are separated from the rest (where customization wouldn't make sense).

Thanks for the catch!

I've decided to seal it anyways, since you can do whatever you want using the combinators.

[fogti]

is it really a good idea to require an allocation for usage? (why not make it generic in such a way that T is a function itself, which would be more general and probably also faster)

[notgull]

Unfortunately it's necessary. This structure is stored directly inside of Queue<T>, so we can't use the function as a parameter without it propagating all the way to Event and EventListener.

In addition, this is on a cold path, so it's unlikely to happen in the first place.

[fogti]

hm so why can't we parameterize Event and EventListener with that anyways and create type aliases for this case? It would blow up the types a bit, but I don't think it would be bad in practical application

[notgull]

The closure stored in these boxes isn't a user provided closure; it's this closure. Until we have TAIT, we can't actually get a type for this closure, therefore we have to box it for now.

[fogti]

Ohhh... but why do we have to use such a closure and not package it into a custom struct and idk... have it implement Iterator or such (or why not just a counter and a generating function?)

[notgull]

You're right. I've replaced the boxed closure with a Vec of tags. It should have the same effect.

[notgull]

The MSRV failure looks spurious to me; running cargo +1.39 build and cargo +1.39 build --no-default-features both work on my machine.

[fogti]

I'm not like 100% sure about the design, but it looks ok to me now.