[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [MirageOS-devel] Using Result instead of Option in libraries

Using polymorphic variants for the error type poses some challenges. Here are some things to think about. (The following code assumes `open Async.Std` because it already has the necessary combinators).

Error Unification

# let x = return (Error `Bad);;
val x : ('a, [> `Bad ]) Result.t Deferred.t = <abstr>

# let f ok = if ok then return (Ok "all good") else return (Error `Error);;
val f : bool -> (string, [> `Error ]) Result.t Deferred.t = <fun>

# x >>=? f;;
- : (string, [> `Bad | `Error ]) Result.t = Core_kernel.Std.Result.Error `Bad

So that works nicely. Two different error types get unified. Notice the inferred types of x and f are open. Forcing a closed type breaks things:

# let x : (bool, [`Bad]) Result.t Deferred.t = return (Error `Bad);;
val x : (bool, [ `Bad ]) Result.t Deferred.t = <abstr>

# x >>=? f;;
Error: This _expression_ has type                                                                                                                      bool -> (string, [> `Error ] as 'a) Result.t Deferred.t                                                                                   but an _expression_ was expected of type                                                                                               
         bool -> (string, [ `Bad ]) Deferred.Result.t
       Type (string, 'a) Result.t Deferred.t is not compatible with type
         (string, [ `Bad ]) Deferred.Result.t =
           (string, [ `Bad ]) Result.t Deferred.t 
       The second variant type does not allow tag(s) `Error

So is the annotation on x wrong? Well, it type checks, so it's not disallowed. But is it just bad style? Is there never a reason to force a closed type? Could adding functions like Bunzli's open_error_msg resolve the matter easily enough?

Unification At Scale

In large code bases, you can't get around the problem because you'll surely introduce incompatible types.

# let x = return (Error (`Bad "uh oh"));;
val x : ('a, [> `Bad of string ]) Result.t Deferred.t = <abstr>

# let f ok = if ok then return (Ok "all good") else return (Error (`Bad 42));;
val f : bool -> (string, [> `Bad of int ]) Result.t Deferred.t = <fun>

# x >>=? f;;
Error: This _expression_ has type                                                                                                                      bool -> (string, [> `Bad of int ] as 'a) Result.t Deferred.t                                                                              but an _expression_ was expected of type                                                                                               
         bool -> (string, [> `Bad of string ] as 'b) Deferred.Result.t
       Type (string, 'a) Result.t Deferred.t is not compatible with type
         (string, 'b) Deferred.Result.t = (string, 'b) Result.t Deferred.t 
       Types for tag `Bad are incompatible

Now you have to manually handle the errors every time x and f are used in the same context. Should some support for that be standardized? We started defining functions like

val bind_and_handle_error
  :  ('a, 'e1) Result.t Deferred.t
  -> ('a -> ('b, 'e2) Result.t Deferred.t
  -> merge_error:('e1 -> 'e2 -> 'e3)
  -> ('a, 'e3) Result.t Deferred.t

It's 3 arguments, so no more infix operator. If you end up with bind_and_handle_error everywhere instead of (>>=?), you soon get annoyed.

Writing MLI Files

The goal is to maintain precise error information, so you diligently define unique variants for all your functions. Now you've got all kinds of polymorphic variant types, getting unified, and growing into larger and larger types. Every function's signature is thus something like

(int, [`Err1 of int | `Err2 of string | .... | `Err20 of unit] Result.t

Type inference gives you this error the first time, but now you copy/paste into your mli. When some upstream code changes, your module no longer compiles because the error type changed. So you have to keep copying/pasting the updated error type. That gets annoying very fast.

An idea to avoid that is to name your error types:

module A : sig
  type err1 = [`Err1 of string]
  val f : int -> (string, err1) Result.t Deferred.t

module B : sig
  type err2 = [`Err2 of int]
  val g : unit -> (string, [err1 | err2]) Result.t Deferred.t

Now, even if you change type err1, the signature for module B remains valid. However, you've introduced a different annoyance; you have to name all your error types. Also, you've only avoided one case. If g's implementation is modified to start using yet another function, then you do have to change g's signature. Note this is very different from the usual case. Normally, calling out to additional code in a function's implementation doesn't change that function's return type. Now you're in a situation where return types change all the time. Worse, if g's implementation stops using f, the signature of g should be changed but the compiler won't tell you that. Quickly you'll have functions that say they can return error `Foo but actually they cannot.

Coding Expertise

Historically beginners have been advised to avoid polymorphic variants. Although this seems to be less true now on, it is certainly the case that coding with them is a bit harder. Example:

With this style, you end up wanting to define a function like:

val with_file :
     string ->
     f : (t -> (‘a, ‘b) Result.t ->
     (‘a, [> err | ‘b]) Result.t

The idea is that opening the file might return an error err. If the file is opened successfully, you call f, and f might return error `b. Thus, the result type should have errors [err | `b]. However, the above doesn't type check. The signature actually has to be (thanks to Sebastien Mondet for teaching me this years ago):

val with_file :
     string ->
     f : (t -> (‘a, [> err] as ‘b) Result.t ->
     (‘a, ‘b) Result.t

Do you want your user and developer base to deal with such signatures? It is unclear what this signature is saying. We now have err mentioned in the return type of f, but in our original intention err has nothing to do with f. Of course, there is an explanation that makes sense of it (but I'll skip it).

I seem to be arguing strongly against the use of polymorphic variants, but I'm not. I'm really unsure what the right answer is. A lot of the problems I'm mentioning would go away if there was a project wide standardization of the particular polymorphic variants used. The value of using them is high, so actually I hope they will be used.

On Fri, Oct 14, 2016 at 10:50 AM, Richard Mortier <richard.mortier@xxxxxxxxxxxx> wrote:
On 14 October 2016 at 15:36, David Scott <scott.dj@xxxxxxxxx> wrote:
>> Looks like we have a number of different conventions for the binds.  Do we
>> want to have the same set of operators with and without Lwt support (and
>> open the Infix module locally as needed) or separate operators that be used
>> alongside each other?
> I'm curious what people recommend here -- I'm certainly open to adopting a
> common convention even if it involves a bit of churn.

Purely personal anecdotal opinion:
FWIW the only addition to the standard Lwt use of `>>=` that I've
found intuitive / memorable enough (somehow) is `>>=?` for an operator
that handles Result types as described too.

(TBH I have a hard time even keeping the meanings of `>>|=` / `>>|?`
in my head.)

Richard Mortier

MirageOS-devel mailing list

MirageOS-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.