{"action":"created","issue":{"url":"https://api.github.com/repos/ocaml/ocaml/issues/147","labels_url":"https://api.github.com/repos/ocaml/ocaml/issues/147/labels{/name}","comments_url":"https://api.github.com/repos/ocaml/ocaml/issues/147/comments","events_url":"https://api.github.com/repos/ocaml/ocaml/issues/147/events","html_url":"https://github.com/ocaml/ocaml/pull/147","id":59519959,"number":147,"title":"added result type","user":{"login":"yminsky","id":715302,"avatar_url":"https://avatars.githubusercontent.com/u/715302?v=3","gravatar_id":"","url":"https://api.github.com/users/yminsky","html_url":"https://github.com/yminsky","followers_url":"https://api.github.com/users/yminsky/followers","following_url":"https://api.github.com/users/yminsky/following{/other_user}","gists_url":"https://api.github.com/users/yminsky/gists{/gist_id}","starred_url":"https://api.github.com/users/yminsky/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yminsky/subscriptions","organizations_url":"https://api.github.com/users/yminsky/orgs","repos_url":"https://api.github.com/users/yminsky/repos","events_url":"https://api.github.com/users/yminsky/events{/privacy}","received_events_url":"https://api.github.com/users/yminsky/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2015-03-02T18:15:08Z","updated_at":"2015-03-02T19:41:24Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/ocaml/ocaml/pulls/147","html_url":"https://github.com/ocaml/ocaml/pull/147","diff_url":"https://github.com/ocaml/ocaml/pull/147.diff","patch_url":"https://github.com/ocaml/ocaml/pull/147.patch"},"body":"This PR adds a new result type, with the following signature:\r\n\r\n    type 'a result = Ok of 'a | Error of 'b\r\n\r\nto Pervasives.  This is very similar to a type that shows up in many\r\nexisting libraries, and should hopefully allow us to converge on one\r\ndefinition that can be shared.  Here are some other known use-cases in\r\nprominent released libraries.\r\n\r\n- Daniel Bunzli's libraries, as separated out in his\r\n  [Result][bunzli-result] library\r\n- Containers' [CCError][containers] type\r\n- Mondet's [pvem][pvem] library\r\n- Batteries' [batResult][batteries-result] type\r\n- Core's [Result][core-result] type\r\n\r\nThe first three of these use the polymorphic variant type,\r\n\r\n    [`Ok of 'a | `Error of 'b]\r\n    \r\nBatteries uses an ordinary variant with this layout:\r\n\r\n    type ('a,'b) t = Ok of 'a | Bad of 'b\r\n    \r\nCore_kernel uses an ordinary variant with this layout:\r\n\r\n    type ('a,'b) t = Ok of 'a | Error of 'b\r\n\r\nAfter some discussion (notably including Daniel Bunzli), it seems like\r\nthe ordinary variant approach is better from a perspective of the less\r\nerror prone typing discipline of ordinary variants.  In particular,\r\nOCaml correctly detects that the following code is broken:\r\n\r\n    type ('a,'b) result =\r\n    | Ok of 'a\r\n    | Error of 'b\r\n\r\n    module Z : sig\r\n      val f : ('a list,'b) result -> int\r\n    end = struct\r\n      let f = function\r\n        | Ok [] -> 0\r\n        | Ok (_::_) -> 1\r\n        | (OK | Error _) -> 2\r\n    end\r\n\r\nBut lets the same code done with polymorphic variants compile without\r\nissues.\r\n\r\n    type ('a,'b) presult =\r\n     [ `Ok of 'a\r\n     | `Error of 'b ]\r\n\r\n    module Z : sig\r\n      val f : ('a list,'b) presult -> int\r\n    end = struct\r\n      let f = function\r\n        | `Ok [] -> 0\r\n        | `Ok (_::_) -> 1\r\n        | (`OK | `Error _) -> 2\r\n    end\r\n\r\nThe hope is that by putting one of these in the stdlib, we can have\r\nbetter interoperability between libraries on what seems to be a quite\r\nbasic type, without having to use the more complex and error prone\r\npolymorphic variant solution.  Bunzli among others have expressed some\r\nconcern with needing to pull in a separate package for such a basic\r\ntype, which is what motivates moving it to the stdlib.\r\n\r\n[batteries-result]: https://github.com/ocaml-batteries-team/batteries-included/blob/master/src/batResult.mli\r\n[core-result]: https://github.com/janestreet/core_kernel/blob/master/lib/result.mli\r\n[bunzli-result]: http://erratique.ch/repos/result/tree/src/result.mli\r\n[containers]: https://github.com/c-cube/ocaml-containers/blob/master/src/core/CCError.mli\r\n[pvem]: https://bitbucket.org/smondet/pvem\r\n"},"comment":{"url":"https://api.github.com/repos/ocaml/ocaml/issues/comments/76795485","html_url":"https://github.com/ocaml/ocaml/pull/147#issuecomment-76795485","issue_url":"https://api.github.com/repos/ocaml/ocaml/issues/147","id":76795485,"user":{"login":"yminsky","id":715302,"avatar_url":"https://avatars.githubusercontent.com/u/715302?v=3","gravatar_id":"","url":"https://api.github.com/users/yminsky","html_url":"https://github.com/yminsky","followers_url":"https://api.github.com/users/yminsky/followers","following_url":"https://api.github.com/users/yminsky/following{/other_user}","gists_url":"https://api.github.com/users/yminsky/gists{/gist_id}","starred_url":"https://api.github.com/users/yminsky/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yminsky/subscriptions","organizations_url":"https://api.github.com/users/yminsky/orgs","repos_url":"https://api.github.com/users/yminsky/repos","events_url":"https://api.github.com/users/yminsky/events{/privacy}","received_events_url":"https://api.github.com/users/yminsky/received_events","type":"User","site_admin":false},"created_at":"2015-03-02T19:41:24Z","updated_at":"2015-03-02T19:41:24Z","body":"In my mind, result is such a close parallel to option, that it strikes me as a truly natural fit with stdlib, and I think it makes sense there.  I agree that we'll need a compatibility library for older compiler versions, but there is some unifying effect of putting things in the stdlib, and I think that would be a valuable contribution on its own."}}