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

Option-returning variants of stdlib functions #885

Merged
merged 13 commits into from Nov 7, 2016

Conversation

Projects
None yet
9 participants
@alainfrisch
Contributor

alainfrisch commented Nov 2, 2016

Stdlib uses exceptions ([Not_found], [Failure], [Invalid_argument]) to report non-exceptional conditions, such failed conversions from strings to numbers, or failing lookups in datastructures. This is commonly thought as a design mistake (esp. because exceptions are not captured in types), and also has potentially bad performance consequences (allocating a [Some] block is usually faster than setting up an exception handler; and raising exception can be really slow when stacktraces are enabled). Also, using exceptions can destroy previous stack traces if those functions are used within exception handlers. Another arguments is that exceptions don't play very nicely with monadic abstraction libraries.

Considering how "brittle" exceptions are, it is best to have users explicitly use them if they want rather than have basic stdlib function raise them.

This PR adds option-returning variants of existing functions. It does not attempt to cover all functions, since in particular [Failure]/[Invalid_argument] have arguably more legitimate uses (see commit messages for some more details on that).

alainfrisch added some commits Nov 2, 2016

Provide a xxx_opt alternative for stdlib functions raising Not_found.
The only exception is the rarely used Buffer.add_substitute, where
the [Not_found] can really be interpreted as an error condition.

Most new functions are implemented directly (instead of wrapping the
raising version).  This is for performance reasons and also to avoid
destroying the stacktrace (if the function is used in an exception
handler).  One could instead implement the raising versions on top of
the new functions, but there might be a small penalty.  Since code
duplication remains reasonable, I think the current state is ok.

Sys.getenv is still implemented on top of the raising version
(itself a runtime primitive).
Provide option-returning variants for Failure/Invalid_arg raising fun…
…ctions.

This adds option-returning variants for some stdlib functions currently
raising Failure or Invalid_arg.

This comments does not try to be exhaustive in providing non-raising
versions.  For instance, invalid ranges in arrays/strings are
considered as programming errors (they can be checked explicitly
by the caller in constant time).  And so are errors related to
applying binary iterators on containers with different sizes.

(I'm not sure about List.hd/List.tl: one could argue that matching
on the list is as simple as matching the result of hd_opt/tl_opt.)

otherlibs/ is not covered by this commit.
@dra27

This comment has been minimized.

Show comment
Hide comment
@dra27

dra27 Nov 2, 2016

Contributor

nativeint.ml, int32.ml and int64.ml should all have the TODO comment for different primitives (as in sys.mlp and pervasives.ml)

Contributor

dra27 commented Nov 2, 2016

nativeint.ml, int32.ml and int64.ml should all have the TODO comment for different primitives (as in sys.mlp and pervasives.ml)

Show outdated Hide outdated stdlib/list.mli
[Failure "tl"] if the list is empty. *)
[Failure "tl"] if the list is empty. *)
val tl_opt: 'a list -> 'a list option

This comment has been minimized.

@dra27

dra27 Nov 2, 2016

Contributor

I'm more strongly convinced that hd_opt and tl_opt are a bad idea than you were! hd and tl should only be used in the "get" scenario (i.e. where you know that the list is non-empty), otherwise you should be pattern matching the list (always), surely? Having hd_opt and tl_opt would seem to encourage poor constructs like:

match List.hd_opt l with
  Some hd -> ...
| None -> ...

or even worse:

List.hd_opt l |> Option.get |> ...

rather than the obvious:

match l with
  hd::_ -> ...
| [] -> ...

Perhaps a change to the docs for hd and tl explaining that philosophy, rather than these two functions? Or is there a really good scenario for wanting to use the _opt versions which I'm not seeing?

@dra27

dra27 Nov 2, 2016

Contributor

I'm more strongly convinced that hd_opt and tl_opt are a bad idea than you were! hd and tl should only be used in the "get" scenario (i.e. where you know that the list is non-empty), otherwise you should be pattern matching the list (always), surely? Having hd_opt and tl_opt would seem to encourage poor constructs like:

match List.hd_opt l with
  Some hd -> ...
| None -> ...

or even worse:

List.hd_opt l |> Option.get |> ...

rather than the obvious:

match l with
  hd::_ -> ...
| [] -> ...

Perhaps a change to the docs for hd and tl explaining that philosophy, rather than these two functions? Or is there a really good scenario for wanting to use the _opt versions which I'm not seeing?

This comment has been minimized.

@alainfrisch

alainfrisch Nov 2, 2016

Contributor

Agreed. I removed these functions.

@alainfrisch

alainfrisch Nov 2, 2016

Contributor

Agreed. I removed these functions.

alainfrisch added some commits Nov 2, 2016

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Nov 2, 2016

Contributor

nativeint.ml, int32.ml and int64.ml should all have the TODO comment

Thanks, done.

Contributor

alainfrisch commented Nov 2, 2016

nativeint.ml, int32.ml and int64.ml should all have the TODO comment

Thanks, done.

@alainfrisch alainfrisch added the stdlib label Nov 2, 2016

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Nov 2, 2016

Contributor

Some timings for comparing performances between options and exceptions: #869 (comment)

Contributor

alainfrisch commented Nov 2, 2016

Some timings for comparing performances between options and exceptions: #869 (comment)

@btj

This comment has been minimized.

Show comment
Hide comment
@btj

btj Nov 2, 2016

Contributor

I guess Num/Big_int isn't strictly part of stdlib, but here's a request to also do int_of_big_int, while you're at it.

Contributor

btj commented Nov 2, 2016

I guess Num/Big_int isn't strictly part of stdlib, but here's a request to also do int_of_big_int, while you're at it.

@dra27

This comment has been minimized.

Show comment
Hide comment
@dra27

dra27 Nov 2, 2016

Contributor

I know it's still work-in-progress, so just to get it on a list: the changes to pervasives.ml need to go to otherlibs/threads/pervasives.ml (blocking Travis) and the relevant testsuite tests for these functions should be duplicated to test the _opt variants too.

Contributor

dra27 commented Nov 2, 2016

I know it's still work-in-progress, so just to get it on a list: the changes to pervasives.ml need to go to otherlibs/threads/pervasives.ml (blocking Travis) and the relevant testsuite tests for these functions should be duplicated to test the _opt variants too.

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Nov 2, 2016

Contributor

Num/Big_int

I've added _opt variants for conversions function in the nums library. That said, the doc did not say explicitly which functions could raise (see final section in num.mli), and I did a very quick pass to figure that out. (I've added a TODO note to add some documentation to this section; it's not just error condition that are not specified.)

Contributor

alainfrisch commented Nov 2, 2016

Num/Big_int

I've added _opt variants for conversions function in the nums library. That said, the doc did not say explicitly which functions could raise (see final section in num.mli), and I did a very quick pass to figure that out. (I've added a TODO note to add some documentation to this section; it's not just error condition that are not specified.)

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Nov 2, 2016

Contributor

the relevant testsuite tests for these functions should be duplicated to test the _opt variants too.

I don't see any test for int_of_string (in particular its error condition) or for List.find. But point taken: all new functions will have to be tested.

Contributor

alainfrisch commented Nov 2, 2016

the relevant testsuite tests for these functions should be duplicated to test the _opt variants too.

I don't see any test for int_of_string (in particular its error condition) or for List.find. But point taken: all new functions will have to be tested.

alainfrisch added some commits Nov 2, 2016

@dra27

This comment has been minimized.

Show comment
Hide comment
@dra27

dra27 Nov 2, 2016

Contributor

I was thinking things like Map.max_binding being copied to Map.max_binding_opt ... but I was also thinking that more of those new functions would have corresponding tests for the old version than it turns out are actually in the testsuite!! I guess being able to build a working compiler probably provides quite good coverage of large parts of the stdlib anyway 😄

Contributor

dra27 commented Nov 2, 2016

I was thinking things like Map.max_binding being copied to Map.max_binding_opt ... but I was also thinking that more of those new functions would have corresponding tests for the old version than it turns out are actually in the testsuite!! I guess being able to build a working compiler probably provides quite good coverage of large parts of the stdlib anyway 😄

@mshinwell

This comment has been minimized.

Show comment
Hide comment
@mshinwell

mshinwell Nov 2, 2016

Contributor

I think it's worth pointing out that the average-case performance of allocation versus an exception trap installation can be misleading. The tails of that distribution are bad; you might trigger a GC and trash your cache. As such, for some applications, the exception-raising versions may be more satisfactory.

I agree that the option-returning versions should be added in any case. The right "performance" thing to do, for functions that return options that cannot be inlined for whatever reason, is probably not to use the exception-returning versions but to allow non-inlined functions to return values of variant type without boxing (and then use the option-returning versions). We are working on that (we had a prototype a while back, but we're waiting until we've finished some structural changes to Flambda before polishing it up).

Contributor

mshinwell commented Nov 2, 2016

I think it's worth pointing out that the average-case performance of allocation versus an exception trap installation can be misleading. The tails of that distribution are bad; you might trigger a GC and trash your cache. As such, for some applications, the exception-raising versions may be more satisfactory.

I agree that the option-returning versions should be added in any case. The right "performance" thing to do, for functions that return options that cannot be inlined for whatever reason, is probably not to use the exception-returning versions but to allow non-inlined functions to return values of variant type without boxing (and then use the option-returning versions). We are working on that (we had a prototype a while back, but we're waiting until we've finished some structural changes to Flambda before polishing it up).

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Nov 2, 2016

Contributor

I've added a test for most new functions (not covered: Ephemerons, *Labels modules, Bytes (but String is covered), and the nums library).

Contributor

alainfrisch commented Nov 2, 2016

I've added a test for most new functions (not covered: Ephemerons, *Labels modules, Bytes (but String is covered), and the nums library).

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Nov 2, 2016

Contributor

Let's wait for a few days to give time to comment on that, but this has received support from two other core developers, so this is on good track for inclusion. If someone objects to the change, please speak up!

Contributor

alainfrisch commented Nov 2, 2016

Let's wait for a few days to give time to comment on that, but this has received support from two other core developers, so this is on good track for inclusion. If someone objects to the change, please speak up!

@braibant

This comment has been minimized.

Show comment
Hide comment
@braibant

braibant Nov 3, 2016

Contributor

It seems like a very good change. I have one minor issue though: in Core, the convention is to have e.g., find and find_exn where find returns an option, and find_exn raises. Now, in the stdlib, we will have find that raises, and find_opt which returns an option. Would it make sense to start introducing the exn suffix in the stdlib (as aliases to existing functions) for consistency? (I would actually be in favor of dropping the existing non-suffixed functions and have the user pick between find_exn and find_opt but that's probably going too far.)

Contributor

braibant commented Nov 3, 2016

It seems like a very good change. I have one minor issue though: in Core, the convention is to have e.g., find and find_exn where find returns an option, and find_exn raises. Now, in the stdlib, we will have find that raises, and find_opt which returns an option. Would it make sense to start introducing the exn suffix in the stdlib (as aliases to existing functions) for consistency? (I would actually be in favor of dropping the existing non-suffixed functions and have the user pick between find_exn and find_opt but that's probably going too far.)

@braibant

This comment has been minimized.

Show comment
Hide comment
@braibant

braibant Nov 3, 2016

Contributor

I would be very mildly in favor of having hd_opt and tl_opt, because those feel nice to have when one is using the option monad (even if the performance argument does not apply there)

Contributor

braibant commented Nov 3, 2016

I would be very mildly in favor of having hd_opt and tl_opt, because those feel nice to have when one is using the option monad (even if the performance argument does not apply there)

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Nov 3, 2016

Contributor

Would it make sense to start introducing the exn suffix in the stdlib (as aliases to existing functions) for consistency?

I'm not a big fan of the _exn variants as found in Core. I'd rather have variants that raise Invalid_argument (instead of Failure/Not_found) for cases where the programmer knows that the operation must succeed, and encourage users to use the option-raising variant in other cases.

Contributor

alainfrisch commented Nov 3, 2016

Would it make sense to start introducing the exn suffix in the stdlib (as aliases to existing functions) for consistency?

I'm not a big fan of the _exn variants as found in Core. I'd rather have variants that raise Invalid_argument (instead of Failure/Not_found) for cases where the programmer knows that the operation must succeed, and encourage users to use the option-raising variant in other cases.

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Nov 3, 2016

Contributor

I would be very mildly in favor of having hd_opt and tl_opt, because those feel nice to have when one is using the option monad (even if the performance argument does not apply there)

Point taken. In order to simplify the discussion, I suggest to leave that out for now, if you are ok? These functions can always be added later.

Contributor

alainfrisch commented Nov 3, 2016

I would be very mildly in favor of having hd_opt and tl_opt, because those feel nice to have when one is using the option monad (even if the performance argument does not apply there)

Point taken. In order to simplify the discussion, I suggest to leave that out for now, if you are ok? These functions can always be added later.

@OvermindDL1

This comment has been minimized.

Show comment
Hide comment
@OvermindDL1

OvermindDL1 Nov 3, 2016

rather have variants that raise Invalid_argument

An Invalid_argument exception makes more sense to me as it is truly an invalid argument (you should have either known or checked before-hand).

OvermindDL1 commented Nov 3, 2016

rather have variants that raise Invalid_argument

An Invalid_argument exception makes more sense to me as it is truly an invalid argument (you should have either known or checked before-hand).

@mshinwell

This comment has been minimized.

Show comment
Hide comment
@mshinwell

mshinwell Nov 3, 2016

Contributor

We probably shouldn't have things that raise Invalid_argument until there is a satisfactory solution for code sensitive to GC latency and flambda use is widespread (raising Not_found will not allocate, but Invalid_argument will unless flambda is being used and the string is a constant).

Contributor

mshinwell commented Nov 3, 2016

We probably shouldn't have things that raise Invalid_argument until there is a satisfactory solution for code sensitive to GC latency and flambda use is widespread (raising Not_found will not allocate, but Invalid_argument will unless flambda is being used and the string is a constant).

@dbuenzli

This comment has been minimized.

Show comment
Hide comment
@dbuenzli

dbuenzli Nov 3, 2016

Contributor

(raising Not_found will not allocate, but Invalid_argument will unless flambda is being used and the string is a constant).

Your code is not supposed to raise Invalid_argument. If that happens your program has a bug.

Contributor

dbuenzli commented Nov 3, 2016

(raising Not_found will not allocate, but Invalid_argument will unless flambda is being used and the string is a constant).

Your code is not supposed to raise Invalid_argument. If that happens your program has a bug.

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Nov 3, 2016

Contributor

I don't think anybody suggest to change the behavior of the current functions (at best they could be gradually deprecated).

Actually, I think we should focus on adding the *_opt variants only for now.

Contributor

alainfrisch commented Nov 3, 2016

I don't think anybody suggest to change the behavior of the current functions (at best they could be gradually deprecated).

Actually, I think we should focus on adding the *_opt variants only for now.

@lpw25

This comment has been minimized.

Show comment
Hide comment
@lpw25

lpw25 Nov 3, 2016

Contributor

I agree with Alain that for now it is best to just focus on adding the *_opt variants.

But I did want to say that it is useful to have both "_exn" variants and "Invalid_argument" variants of functions. The first kind should use precise exceptions (e.g. Not_found) and should only be used for performance reasons. The second kind should use generic exceptions (e.g. Invalid_arguments) and IMO should also have a different name (e.g. get instead of find): they are used for cases where only a programming error could result in the exception being raised (and as such it is not intended to be caught).

(And if typed effects ever get into OCaml then the _exn variants would be deprecated in favour of _eff variants which perform faster and are safer, whilst the Invalid_arguments variants would remain).

Contributor

lpw25 commented Nov 3, 2016

I agree with Alain that for now it is best to just focus on adding the *_opt variants.

But I did want to say that it is useful to have both "_exn" variants and "Invalid_argument" variants of functions. The first kind should use precise exceptions (e.g. Not_found) and should only be used for performance reasons. The second kind should use generic exceptions (e.g. Invalid_arguments) and IMO should also have a different name (e.g. get instead of find): they are used for cases where only a programming error could result in the exception being raised (and as such it is not intended to be caught).

(And if typed effects ever get into OCaml then the _exn variants would be deprecated in favour of _eff variants which perform faster and are safer, whilst the Invalid_arguments variants would remain).

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Nov 3, 2016

Contributor

The first kind should use precise exceptions (e.g. Not_found) and should only be used for performance reasons.

Are you referring to the case reported by @mshinwell ("The tails of that distribution are bad; you might trigger a GC and trash your cache.")?

Are you aware of plans (in flambda) to support non-regular calling conventions that would allow (amongst others) unboxed option as function results (and arguments)? This would probably be much faster than exceptions, even for the tail of the distribution.

Contributor

alainfrisch commented Nov 3, 2016

The first kind should use precise exceptions (e.g. Not_found) and should only be used for performance reasons.

Are you referring to the case reported by @mshinwell ("The tails of that distribution are bad; you might trigger a GC and trash your cache.")?

Are you aware of plans (in flambda) to support non-regular calling conventions that would allow (amongst others) unboxed option as function results (and arguments)? This would probably be much faster than exceptions, even for the tail of the distribution.

@lpw25

This comment has been minimized.

Show comment
Hide comment
@lpw25

lpw25 Nov 3, 2016

Contributor

Are you aware of plans (in flambda) to support non-regular calling conventions that would allow (amongst others) unboxed option as function results (and arguments)?

Yes, but until they exist and are part of the default compilation option those functions will still be useful.

Even with those optimisations exceptions would probably still be faster in some cases (e.g. letting the Not_found bubble up to the top of a recursion) although those are probably not common enough to justify having the extra functions. (Of course in this case the "extra" functions are the ones we are stuck with for backwards compatibility anyway).

Contributor

lpw25 commented Nov 3, 2016

Are you aware of plans (in flambda) to support non-regular calling conventions that would allow (amongst others) unboxed option as function results (and arguments)?

Yes, but until they exist and are part of the default compilation option those functions will still be useful.

Even with those optimisations exceptions would probably still be faster in some cases (e.g. letting the Not_found bubble up to the top of a recursion) although those are probably not common enough to justify having the extra functions. (Of course in this case the "extra" functions are the ones we are stuck with for backwards compatibility anyway).

@lpw25

This comment has been minimized.

Show comment
Hide comment
@lpw25

lpw25 Nov 3, 2016

Contributor

Even with those optimisations exceptions would probably still be faster in some cases

Also when I say faster, I mean only very slightly faster

Contributor

lpw25 commented Nov 3, 2016

Even with those optimisations exceptions would probably still be faster in some cases

Also when I say faster, I mean only very slightly faster

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Nov 3, 2016

Contributor

Even with those optimisations exceptions would probably still be faster in some cases (e.g. letting the Not_found bubble up to the top of a recursion)

Yes, but the same would be achieved by letting users do a more explicit match ... with Some x -> x | None -> raise Not_found (assuming an optimized compilation that gets rid of the option); and then probably use a locally defined exception than a generic Not_found.

Contributor

alainfrisch commented Nov 3, 2016

Even with those optimisations exceptions would probably still be faster in some cases (e.g. letting the Not_found bubble up to the top of a recursion)

Yes, but the same would be achieved by letting users do a more explicit match ... with Some x -> x | None -> raise Not_found (assuming an optimized compilation that gets rid of the option); and then probably use a locally defined exception than a generic Not_found.

@lpw25

This comment has been minimized.

Show comment
Hide comment
@lpw25

lpw25 Nov 3, 2016

Contributor

It was the cost of returning from the function and branching before the exception raise which I was thinking of. I did say "only very slightly" faster. Of course if the call is inlined then even these tiny costs disappear.

The other thing I was thinking of was cases where the function being called was not statically known and so could not be optimised.

But anyway, I agree that _exn is probably not needed once flambda can unbox variants across function boundaries (and once flambda is on by default).

Contributor

lpw25 commented Nov 3, 2016

It was the cost of returning from the function and branching before the exception raise which I was thinking of. I did say "only very slightly" faster. Of course if the call is inlined then even these tiny costs disappear.

The other thing I was thinking of was cases where the function being called was not statically known and so could not be optimised.

But anyway, I agree that _exn is probably not needed once flambda can unbox variants across function boundaries (and once flambda is on by default).

@alainfrisch alainfrisch changed the title from [WIP] Option-returning variants of stdlib functions to Option-returning variants of stdlib functions Nov 7, 2016

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Nov 7, 2016

Contributor

Since there is rather strong support for this and no opposition, I think this can be merged. I've heard that the best practice is that the contributor does not click on "Merge" himself, so I'll let the honor to another core developer ( @dra27 ? ).

Contributor

alainfrisch commented Nov 7, 2016

Since there is rather strong support for this and no opposition, I think this can be merged. I've heard that the best practice is that the contributor does not click on "Merge" himself, so I'll let the honor to another core developer ( @dra27 ? ).

@dra27

This comment has been minimized.

Show comment
Hide comment
@dra27

dra27 Nov 7, 2016

Contributor

Sure - just for paranoia's sake, I shall wait for Travis to be green (though I think it's only checking the Changes file!)

Contributor

dra27 commented Nov 7, 2016

Sure - just for paranoia's sake, I shall wait for Travis to be green (though I think it's only checking the Changes file!)

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Nov 7, 2016

Contributor

It's green now, the Changes file was ok 😄

Contributor

alainfrisch commented Nov 7, 2016

It's green now, the Changes file was ok 😄

@dra27

This comment has been minimized.

Show comment
Hide comment
@dra27

dra27 Nov 7, 2016

Contributor

One further thought - do you want to fix up the commits or just squash the whole thing into one?

Contributor

dra27 commented Nov 7, 2016

One further thought - do you want to fix up the commits or just squash the whole thing into one?

@alainfrisch

This comment has been minimized.

Show comment
Hide comment
@alainfrisch

alainfrisch Nov 7, 2016

Contributor

I don't think there is any value in keeping separate commits. I'd suggest to click on "Squash and merge".

Contributor

alainfrisch commented Nov 7, 2016

I don't think there is any value in keeping separate commits. I'd suggest to click on "Squash and merge".

@dra27 dra27 merged commit 69263a9 into ocaml:trunk Nov 7, 2016

2 checks passed

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

This comment has been minimized.

Show comment
Hide comment
@dra27

dra27 Nov 7, 2016

Contributor

Be very afraid 😉

Contributor

dra27 commented Nov 7, 2016

Be very afraid 😉

g2p added a commit to g2p/ocaml that referenced this pull request Nov 8, 2016

Implement find_first, find_last for maps
Finds the first/last binding where the key satisfies a monotonic
predicate, returning an option.

Followup to #885.

g2p added a commit to g2p/ocaml that referenced this pull request Nov 9, 2016

Implement find_first_opt, find_last_opt for maps
Finds the first/last binding where the key satisfies a monotonic
predicate, returning an option.

Followup to #885.

g2p added a commit to g2p/ocaml that referenced this pull request Nov 9, 2016

Implement find_first_opt, find_last_opt for sets
Finds the first/last element that satisfies a monotonic predicate,
returning an option.

Followup to #885.

g2p added a commit to g2p/ocaml that referenced this pull request Nov 9, 2016

Implement find_first_opt, find_last_opt for maps
Finds the first/last binding where the key satisfies a monotonic
predicate, returning an option.

Followup to #885.

g2p added a commit to g2p/ocaml that referenced this pull request Nov 9, 2016

Implement find_first_opt, find_last_opt for sets
Finds the first/last element that satisfies a monotonic predicate,
returning an option.

Followup to #885.

g2p added a commit to g2p/ocaml that referenced this pull request Nov 10, 2016

Implement find_first_opt, find_last_opt for maps
Finds the first/last binding where the key satisfies a monotonic
predicate, returning an option.

Followup to #885.

g2p added a commit to g2p/ocaml that referenced this pull request Nov 10, 2016

Implement find_first_opt, find_last_opt for sets
Finds the first/last element that satisfies a monotonic predicate,
returning an option.

Followup to #885.

g2p added a commit to g2p/ocaml that referenced this pull request Nov 10, 2016

Implement find_first_opt, find_last_opt for maps
Finds the first/last binding where the key satisfies a monotonic
predicate, returning an option.

Followup to #885.

g2p added a commit to g2p/ocaml that referenced this pull request Nov 10, 2016

Implement find_first_opt, find_last_opt for sets
Finds the first/last element that satisfies a monotonic predicate,
returning an option.

Followup to #885.

g2p added a commit to g2p/ocaml that referenced this pull request Nov 12, 2016

Implement find_first_opt, find_last_opt for maps
Finds the first/last binding where the key satisfies a monotonic
predicate, returning an option.

Followup to #885.

g2p added a commit to g2p/ocaml that referenced this pull request Nov 12, 2016

Implement find_first_opt, find_last_opt for sets
Finds the first/last element that satisfies a monotonic predicate,
returning an option.

Followup to #885.

camlspotter pushed a commit to camlspotter/ocaml that referenced this pull request Oct 17, 2017

Option-returning variants of stdlib functions (#885)
Provide an xxx_opt alternative for functions raising Not_found
and many instances of Failure/Invalid_arg.

The only exception is the rarely used Buffer.add_substitute, where
the [Not_found] can really be interpreted as an error condition.

Most new functions are implemented directly (instead of wrapping the
raising version).  This is for performance reasons and also to avoid
destroying the stacktrace (if the function is used in an exception
handler).  One could instead implement the raising versions on top of
the new functions, but there might be a small penalty.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment