List.filter_map

[trefis]

No description provided.

[alainfrisch]

Thanks, this addition has been long overdue!

One question: Is such a function available in Batteries/Core/Containers, and if so, under which name(s)?

One remark: You made the function tail recursive. I think it becomes more and more difficult to justify than List.map, perhaps the most widely used function in List, is not tailrec, especially if all new additions, even those similar in spirit, are. I remember an old argument that the problem was not with List.map, but with users relying on list to hold large collections, but the argument is rather weak. Assuming the system cannot be fixed (e.g. with extensible stacks) to support non tailrec functions on long lists, one should probably aim at ensuring that all functions in List.map work on long lists (without degrading performance on small lists, hopefully).

[trefis]

All of them under the same name:

• Base
• Containers
• Batteries

As for the tailrec discussion: I just copy pasted what was in Misc.Stdlib.List. If there is a general agreement that it should be implemented differently, I don't mind updating the PR. I personally do not care (well, apart from #181, I care about that).

[Chris00]

should be implemented differently,

This should ideally be supported by benchmarks but how about to let it use "direct" recursion up to to a certain depth and, if that depth is exceeded, use your tail-recursive implementation on the remaining tail?

[trefis]

I don't think that any other function in the stdlib does this at the moment, and I don't plan to be the first person to add one like that.

[alainfrisch]

This should ideally be supported by benchmarks but how about to let it use "direct" recursion up to to a certain depth and, if that depth is exceeded, use your tail-recursive implementation on the remaining tail?

Yes, that's the usual way to get a function that works on long lists without the extra cost on small lists (and unrolling the loop a few times more than compensate the overhead of counting elements). One downside, though, is that such implementation might results in loosing inlining opportunities (with flambda) since the callback's body would be duplicated after inlining.

[gasche]

I believe that the implementation is correct, but I would still like to see tests for it.
(testsuite/tests/lib-list/test.ml has tests for list functions.)

[mookid]

I don't think that any other function in the stdlib does this at the moment, and I don't plan to be the first person to add one like that.

init does that.

[trefis]

init does that.

Indeed, I stand corrected.

Anyway: if we want to go with a "simple" version (i.e. either tail-rec or not tail-rec), I'd vote for the tail-rec one, because that's what's already used in the compiler and I don't want to:

• have to implementations of the function lying around
• think about whether these particular calls would be OK with a non-tail-rec version or not.

[Armael]

I think it's already very useful to have this added to the stdlib as-is -- improved implementations using unrolling can happen at a later time, and be done consistently on all the relevant functions at the same time.

[alainfrisch]

Ok, let's go with the tail-rec version. Please add a test, and this is good to merge I think.

[trefis]

Test added.

[trefis]

@alainfrisch feel free to click the "squash and merge" button if you're happy with this :)

[alainfrisch]

I'm happy but I'd suffer @gasche's wrath if I accepted a Changes entry without proper author attribution.

[trefis]

Ah! Indeed. I've updated the entry.