Relax the return types of Eio.Process.pipe

[rizo]

Motivation

Currently the Eio.Process.pipe function has the following type:

val pipe : sw:Switch.t -> _ mgr ->
  [Flow.source_ty | Resource.close_ty] Std.r *
  [Flow.sink_ty | Resource.close_ty] Std.r

This means that the created sources and sinks are required to be "closable". This restriction currently prevents compatibility with types that are monomorphically just sinks or just sources (i.e., without the `Close tag).

My suggestion is to change the type of pipe to:

val pipe : sw:Switch.t -> _ mgr ->
  [< Flow.source_ty | Resource.close_ty] Std.r *
  [< Flow.sink_ty | Resource.close_ty] Std.r

This would allow the resulting sources and sinks to be implicitly used as any of the allowed subtypes.

Examples

To elaborate on why I think this is useful, consider the two examples below.

This works: flexible source consumers

let src, _snk = Eio.Process.pipe ~sw proc_mgr in
Eio.Flow.read_all src

If we expand the read_all type, it accepts [> `R] flows, which of course type-checks because any additional variants, like `Close are allowed. Thus, we can see that read_all is a "good" consumer and does not limit our types.

This does not work: fixed flow types

Some APIs use monomorphic versions of source and sink types. For example, the Cohttp_eio.Body.t type is defined as:

type t = Eio.Flow.source_ty Eio.Resource.t

Which means that the following example does not work:

let src, _snk = Eio.Process.pipe ~sw proc_mgr in
Cohttp_eio.Server.respond ~status:`OK ~body:src

Failing with:

25 |   Cohttp_eio.Server.respond ~status:`OK ~body:src
                                                   ^^^
Error: This expression has type [ `Close | `Flow | `R ] Eio.Resource.t
       but an expression was expected of type
         Cohttp_eio__.Body.t = Eio.Flow.source_ty Eio.Resource.t
       Type [ `Close | `Flow | `R ] is not compatible with type
         Eio.Flow.source_ty = [ `Flow | `R ]
       The second variant type does not allow tag(s) `Close

This is fairly obvious, but unless, I missed something, there's currently no way to "restrict" a [> source_ty] Std.r to just source_ty Std.r in the Eio's API.

We could argue that Cohttp_eio.Body.t, and/or functions that work on this type, should be more permissive and use open types, but, not only I think this is a non-obvious requirement for library authors, it also complicates the public APIs by exposing all the row variables.

Conclusion

By changing the type of Process.pipe to use polymorphic types for source and sinks, we allow them to be compatible with their monomorphic versions.

[rizo]

Forgot to mention, that I do realize that it is possible to explicitly cast any source-like type to just a source with:

let source = (source_like :> Eio.Flow.source_ty Eio.Flow.source)

But, I still think there's value in allowing pipe to return polymorphic types.

[talex5]

Make sense. We already made this change for e.g. Eio_unix.Net.import_socket_stream.

(and the Git version of cohttp-eio also relaxes the type for respond)

[Arogbonlo]

Hello @patricoferris,

I'm Isaac Arogbonlo, and I've been accepted into the Outreachy contribution phase. I would like to work on this issue under your guidance and would be grateful if I could be assigned to it. Thank you.

[patricoferris]

Glad to have you @Arogbonlo -- let me know if you need any help. Feel free to open a PR whenever you are ready/need more input and ping me!

[Arogbonlo]

Thank you @patricoferris I'll update you on any progress and setbacks I have. Cheers!

[patricoferris]

Hey @Arogbonlo -- before I can assign, do you mind making sure you have the project setup (i.e. OCaml installed and Eio building on Windows). This helps with managing the issues if we know you are ready to work on them. Let me know when you are ready and I'll reassign the issue, I'll also update the meta-issue to detail this step. Apologies for the confusion.

[Arogbonlo]

Oh yeah i have those already set up. So I'm ready to begin. Then you could do well to update the meta-issue to detail the step. I believe that would help

[oyenuga17]

@create2000 I think you should pick up another issue while we give @Arogbonlo time to send his screenshot. Here is the link to other good first issues

[create2000]

@oyenuga17 -- yeah, i have picked another issue. Thank you.

[Arogbonlo]

Hey @patricoferris ,This shows I have everything set up!

[oyenuga17]

@Arogbonlo this only shows that you've got Ocaml running your system please attach a screenshot of your terminal running the Eio "Hello World" example in utop.

[oyenuga17]

@Arogbonlo this only shows that you've got Ocaml running your system please attach a screenshot of your terminal running the Eio "Hello World" example in utop.

[Arogbonlo]

here @oyenuga17 @patricoferris

[patricoferris]

Thanks @Arogbonlo -- the first screenshot you sent looked more promising (it was a compatible OCaml version for Eio, which needs OCaml 5+).

Can I just double-check that this is running natively on Windows? It looks a little like WSL but I might be mistaken :))

[Arogbonlo]

Sure. Hope this is good enough. @patricoferris I ensured Ocaml is running version 5.1.0 Previously it wasn't running on windows. But now it is.

[patricoferris]

Perfect, the Eio program isn't running there, but you should just be an opam install . --deps-only --with-test away from fixing that :))

[Arogbonlo]

hey @patricoferris @oyenuga17 after changing the pipe functions to be polymorphic, i still ahve errors returning to me and I've been on this for a good while. So please could you help me out. Below is a copy of the error after running the "dune build".

File "lib_eio/unix/process.ml", line 1:
Error: The implementation lib_eio/unix/process.ml
       does not match the interface lib_eio/unix/.eio_unix.objs/byte/eio_unix__Process.cmi:
        ... ... In module Make_mgr:
       Values do not match:
         val pipe :
           'a ->
           sw:Eio__core.Switch.t ->
           [ `Close | `Flow | `R ] mgr * [ `Close | `Flow | `W ] mgr
       is not included in
         val pipe :
           t ->
           sw:Eio__core.Switch.t ->
           [< `Close | `Flow | `R ] mgr * [< `Close | `Flow | `W ] mgr
       The type
         t ->
         sw:Eio__core.Switch.t ->
         [ `Close | `Flow | `R ] mgr * [ `Close | `Flow | `W ] mgr
       is not compatible with the type
         t ->
         sw:Eio__core.Switch.t ->
         [< `Close | `Flow | `R ] mgr * [< `Close | `Flow | `W ] mgr
       The tag `R is guaranteed to be present in the first variant type,
       but not in the second
       File "lib_eio/process.mli", lines 171-174, characters 4-87:
         Expected declaration
       File "lib_eio/unix/process.ml", line 120, characters 6-10:
         Actual declaration

And I've made the necessary changes to the ./unix/process.ml file

[oyenuga17]

Hey @Arogbonlo I think you'll also need to change the type on the module interface like the error states. you can go to line 171 in process.mli to effect the change.

[Arogbonlo]

thank you so much @oyenuga17 . I'll effect that and give you feedback!

[Arogbonlo]

Hey @oyenuga17 @patricoferris as we can see in the pictures where I relaxed the return type of pipe, however after running the dune build , I still get this error C:\Users\user\eio>dune build File "lib_eio/unix/process.ml", line 1: Error: The implementation lib_eio/unix/process.ml does not match the interface lib_eio/unix/.eio_unix.objs/byte\eio_unix__Process.cmi: ... ... In module Make_mgr: Values do not match: val pipe : 'a -> sw:Eio__core.Switch.t -> Types.source_ty mgr * Types.sink_ty mgr is not included in val pipe : t -> sw:Eio__core.Switch.t -> [>Close | Flow |R ] mgr [> Close |Flow | `W ] mgr The type t -> sw:Eio__core.Switch.t -> Types.source_ty mgr Types.sink_ty mgr is not compatible with the type t -> sw:Eio__core.Switch.t -> [> Close |Flow | R ] mgr * [>Close | Flow |W ] mgr Type Types.source_ty = [ Close |Flow | R |Unix_fd ] is not compatible with type [> Close |Flow | R ] The second variant type does not allow tag(s)Unix_fd File "lib_eio/process.mli", lines 171-174, characters 4-87: Expected declaration File "lib_eio/unix/process.ml", line 120, characters 6-10: Actual declaration`

     Please I'd appreciate your intervention. Thank you

[talex5]

You might want to look at https://github.com/ocaml-multicore/eio/pull/744, which made the same change for sockets.

In particular, take care of the difference between < and >.

[patricoferris]

Hey @Arogbonlo

Thanks for your work. Firstly, do configure your editor to help working with OCaml.

Next, I think it is important to understand what these type errors are saying. To start with, I would read about polymorphic variants in OCaml. In fact, @talex5 has an excellent blog post for just this: https://roscidus.com/blog/blog/2013/12/20/polymorphism-for-beginners/#polymorphic-variants.

Further, here is small test program like the cohttp-eio example but without any dependencies that should type check after your changes:

let read_constrained (src : Eio.Flow.source_ty Eio.Resource.t) =
  Eio.Flow.read_all src

let () =
  Eio_posix.run @@ fun env ->
  let proc = Eio.Stdenv.process_mgr env in
  Eio.Switch.run @@ fun sw ->
  let src, sink = Eio.Process.pipe ~sw proc in
  Eio.Flow.copy_string "hello" sink;
  Eio.Flow.close sink;
  read_constrained src |> print_endline

[Arogbonlo]

You might want to look at #744, which made the same change for sockets.

In particular, take care of the difference between < and >.

Thank you for this. I'll look into this immediately.

[Arogbonlo]

Hey @Arogbonlo

Thanks for your work. Firstly, do configure your editor to help working with OCaml.

Next, I think it is important to understand what these type errors are saying. To start with, I would read about polymorphic variants in OCaml. In fact, @talex5 has an excellent blog post for just this: https://roscidus.com/blog/blog/2013/12/20/polymorphism-for-beginners/#polymorphic-variants.

Further, here is small test program like the cohttp-eio example but without any dependencies that should type check after your changes:

let read_constrained (src : Eio.Flow.source_ty Eio.Resource.t) =
  Eio.Flow.read_all src

let () =
  Eio_posix.run @@ fun env ->
  let proc = Eio.Stdenv.process_mgr env in
  Eio.Switch.run @@ fun sw ->
  let src, sink = Eio.Process.pipe ~sw proc in
  Eio.Flow.copy_string "hello" sink;
  Eio.Flow.close sink;
  read_constrained src |> print_endline

Wow! Having configured my editor to help working with ocaml, its all becoming very interesting and clear. Thanks so much for this @patricoferris

Also, I'll have a good read on the article. I'm sure it would be of great help.

Then, once I'm done, I could use the type case to test.

[Arogbonlo]

Hey @patricoferris @oyenuga17 I just created a PR for this issue. Please do well to check it out.

Thank you