Lwt: Add Lwt_mvar.peek

[hcarty]

A function which would immediately return the current state of the mvar (contents or None if empty).

I've wanted this on several occasions but never filled an issue or submitted a PR. Time to change that!

If such a function would be accepted/considered I'll create a PR for it.

[hcarty]

Val peek : 'a t -> 'a option

And potentially an is_full as well.

[aantron]

It sounds useful to me. Not sure if anyone else has an opinion.

As for is_full, I want to ask about the name – is "full" standard terminology with mvars? If not, I would prefer to reuse some of the terminology already used in lwt_mvar.mli, e.g. empty, available, or please adjust docs in the same PR to keep the number of distinct adjectives low :)

[raphael-proust]

Question: does peek pop the value out if there are any?

Mvars are useful as a synchronisation thing: only one promise can take a value from it. So I would expect peek to pop the value out if there is any.

But the name peek, when used for other datastructures like queues, tend to have the opposite meaning: it returns what pop would return, but doesn't pop the value out.

---

I don't think we should add a read-and-leave-the-value function because it can break some assumption that people make: I took the value from the mvar, I'm the only one with that value.

I'm in favour of a function that can immediately return the content if any. I'd call it something different though. Maybe poll?

---

I think a val empty: 'a t -> bool is good.

[hcarty]

@aantron Very good points on terminology - being inconsistent here doesn't help anyone!

@raphael-proust I agree - better to not break assumptions around 'a Lwt_mvar.t being a once-in, once-out value.

What do you think of take_available as a name, following the naming scheme in Lwt_stream.get_available for a function which takes what it can without blocking? Similarly for is_empty.

val take_available : 'a t -> 'a option
(** [take_available mvar] immediately takes the value from [mvar] without blocking, returning
    [None] if the mailbox is empty. *)

val is_empty : 'a t -> bool
(** [is_empty mvar] indicates if [put mvar] can be called without blocking. *)

[aantron]

I have no objection to these names, except a vague hesitation about the part of the proposal that uses Lwt_stream as example, just because of the Lwt_stream problems and rewrites discussion (#250). But if the name is good for other reasons, great :) Let's see if there are any other opinions, though.

[aantron]

(and it's also good that take_available matches up with take)

[hcarty]

Understood regarding Lwt_stream - I thought that might come up! In this case, though, I don't know that we have another Lwt example to pull from.

[aantron]

It's good to note Lwt_stream anyway, so thanks :)

[raphael-proust]

These function names (and signature) look good.

[raphael-proust]

Is anyone working on this? Otherwise, I'll give it a shot this evening.

[aantron]

I was under the impression @hcarty will submit a PR; @hcarty?

[hcarty]

@raphael-proust @aantron I was going to work on it later this week. If you have time to work on it now/today then please go for it! I'm happy to help with the review in that case.

[raphael-proust]

There's another issue that I'll try to tackle first. If I finish early I'll give this one a stab. Otherwise @hcarty I'll review any commit that you have time to put together.

[aantron]

Thanks for synchronizing :) And I'll of course help review everything :)

[hcarty]

PR in as #459 - sorry it took a while to get to!

[aantron]

No worries, there is no rush. And thanks :)