Optional thunk #170
Optional thunk #170
Conversation
This module implements an option value specialized for thunks. It uses the identity function as a special value to represent none, which means that a "some" value cannot be constructed using the identity function.
This is just used for the ready_to_write callback. The read callbacks are already guarded by a bool.
|
@Lupus can you take this for a spin? |
|
Just checked this branch against our test suite - it works just fine, as it used before the #161! 🎉 Thanks for quick turnaround! |
|
|
||
| let is_none t = t == none | ||
| let is_some t = not (is_none t) | ||
| let unchecked_value t = t |
There was a problem hiding this comment.
I was thinking about this when I was reviewing it and asking myself "should this be raising if t is none?" – but that felt a little burdensome in all of the places we use it safely.
But now I'm thinking about it some more, and I'm wondering: maybe it would make sense if we split this into two functions, kind of like what we talked about before the PR:
(** Return the exposed thunk, raises if called on [none] *)
val value_exn : t -> unit -> unit
(** Invoke the thunk if [some], do nothing if [none] *)
val invoke_if_some : t -> unitThis has two benefits: calling an optional thunk should always be safe, since the default is to just noop by invoking none, and we can use this in all of the current places where we use unchecked_value without is_some. But removing opaqueness is something you have to be a bit more careful about, since presumably you should be able to do Optional_thunk.some (Optional_thunk.value_exn t). And ultimately I rank "descriptive names" below "failing at runtime" and "compiler error" in terms of safety.
Introduce an
Optional_thunk.ttype to replace most of theSys.opaque_identitiesused throughout the code. This type requires one to check whether it is "none" or "some" before using it as a regular function. Theunchecked_valuefunction reminds you that you might be performing an unsafe operation.