Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stdlib: add Fun module. #2129

Merged
merged 2 commits into from Nov 6, 2018

Conversation

@dbuenzli
Copy link
Contributor

commented Nov 1, 2018

This PR adds a Fun module to the standard library. This will prevent the new protect function added by #1855 and being refined in #2118 to be added to the initial scope (aka Stdlib, ex-Pervasives).

A few notes:

  1. The PR provides a few functions that can be systematically found in containers' CCFun module, base's Fn module and the Haskell Prelude. These are exactly: const, id and flip.

  2. While preparing this PR I discovered that the function Bool.negate introduced by PR #2010 actually lives in Base under the name Fn.non. We could consider moving Bool.negate to Fun.negate, it seems to me the latter would might make more sense from a reading point of view. If there is agreement I can add a commit to this PR to implement this.

@alainfrisch
Copy link
Contributor

left a comment

LGTM. (The link to Base's Fn should be https://github.com/janestreet/base/blob/master/src/fn.mli )

To be clear, do you intend to move protect in a later PR, or did you intend the move to be part of this one but forgot about it?

(I suggest to discuss more additions to Fun in subsequent PRs.)

manual/manual/library/stdlib.etex Outdated Show resolved Hide resolved
@dbuenzli

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2018

To be clear, do you intend to move protect in a later PR,

Yes or if this is gets merged before the work on #2118 ends. I expect the latter to do the move directly.

What do you think about point 2. ?

@alainfrisch

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

Re 2: yes, Fun.negate reads better than Bool.negate.

@alainfrisch

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

(In terms of future addition, let me suggest ('a -> 'b) -> ('a -> ('b, exn) Result.t) -- which I would have called protect if the name was not already taken.)

@dbuenzli

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2018

(In terms of future addition, let me suggest ('a -> 'b) -> ('a -> ('b, exn) Result.t) -- which I would have called protect if the name was not already taken.)

I have these things in Rresult here under the name trap_exn, but I'm not sure they are a good idea: they imply a catch all exception handler but some exceptions like Break should not be caught (maybe we can list those though).

@alainfrisch

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

but some exceptions like Break should not be caught

This is a very good reason to expose such a function since it's unlikely that user-defined version would be fully correct. Or perhaps a predicate Exn.catchable: exn -> bool (or whatever).

@dbuenzli dbuenzli force-pushed the dbuenzli:fun-module branch from 4ecef95 to d017c7c Nov 2, 2018

@alainfrisch alainfrisch added the stdlib label Nov 2, 2018

@alainfrisch alainfrisch merged commit 89e48a3 into ocaml:trunk Nov 6, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dbuenzli

This comment has been minimized.

Copy link
Contributor Author

commented Nov 6, 2018

Thanks !

@dbuenzli dbuenzli deleted the dbuenzli:fun-module branch Nov 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.