Add foldlWhile to List.Lazy

[drewolson]

I'm not sure if this is something you'd like to add, but I've found this construct useful in other languages that support explicit lazy lists.

For example, both rust and elixir support something like this function.

[drewolson]

Any thoughts on this one @garyb?

[garyb]

It seems sensible enough to me! I don't really have much of a personal opinion about the Lazy stuff here, since I haven't needed it much.

@hdgarrood / @natefaubion any thoughts?

[hdgarrood]

Yeah, seems fine! No strong opinions either way from me. I don't think I've ever used lazy lists in PureScript 😮

[natefaubion]

Is there a reason to not also add such a function for Data.List? I don't like having API differences between the two unless there's a good reason.

[natefaubion]

Also, isn't this foldl with the arguments flipped?

[natefaubion]

Did you intend to use go here? Otherwise there's no reason for the binding, you can just case on step.

[hdgarrood]

Come to think of it, I think using go here is probably sensible as we can get TCO without having to thread f through for each iteration.

[drewolson]

I didn't intend to use go. The recursive call needs to call step again, so it needs to go back in through the top. I'm happy to remove go completely, though the extraction makes it a bit easier for me to read the implementation (otherwise you'd have nested case statements).

[hdgarrood]

This needs to be set up in such a way that TCO fires or it will overflow for long lists in the case where the function keeps returning Right for long enough. I’d suggest:

foldlWhile f = go
  where
  go acc xs =
    case step xs of
      Nil ->
        acc
      Cons x ys ->
        case f acc x of
          Left acc’ -> acc’
          Right acc’ -> go acc’ ys

Using a binding for go like this means that we don’t need to thread f back through each time.

[drewolson]

@hdgarrood for my edification, why wasn't the initial implementation tail recursive? Is it the mutually recursive call from go back to foldlWhile?

[hdgarrood]

I think your implementation could also have been described as tail-recursive, although I was concerned that introducing the go binding and then recursing with a call to the outer binding would mean it was tail-recursive in a way that the compiler might not be able to recognise and optimise, if that makes sense?

[hdgarrood]

Oh yeah of course, this is folding from the left, so I guess it should be called foldlWhile?

[natefaubion]

I think we should rename it, and flip the arguments back then.

[hdgarrood]

Actually, on second thoughts I'm no longer sure which direction it's folding in. Yes, this is definitely a left fold; see https://stackoverflow.com/questions/13280159/haskell-foldl-and-foldr/13280185

[drewolson]

@hdgarrood my understanding was that folds over lazy lists were almost by definition folds from the right. I'm no expert, though. I'm happy to do any of the following:

• rename to foldrWhile and swap the args
• also add a version of the function to List

Let me know.

[natefaubion]

@drewolson What determines left or right fold is just associativity of the operations, and this is left associative. For it to be right associative you would need to pass in the accumulator for the entire tail to the folding function. A lazy foldr obviates the need for a breaker because you have control over evaluation of the tail when folding. Since evaluation of the tail is deferred, if you never demand it, it's never run, letting you short circuit the computation.

I would like both of those tasks to be done: changing the name/signature, and adding an implementation for Data.List so we can keep API parity.

[hdgarrood]

Renaming to foldlWhile and flipping the function argument sounds good, yes please. Also could you please add a test where the function keeps returning Right for 100,000 iterations before the first Left, to ensure that we have stack safety?

[drewolson]

@hdgarrood @natefaubion I believe I've addressed all concerns. Thanks for all the feedback!

[hdgarrood]

LGTM, thanks!

[hdgarrood]

I'd prefer to keep the internal go function, since a) it means that we won't need to rebind f on each loop, and b) it makes it a little easier to read as you no longer have to worry about f changing.

[drewolson]

@hdgarrood I actually found the indirection of introducing go made the code harder for me to read. That said, if the consensus is that I should re-introduce it, I'm happy to do so.

[drewolson]

Any further thoughts on this? If it isn't something that folks are interested in, I'm happy to close the PR.

[hdgarrood]

It's not that we don't want it, it's just that we're spread fairly thin. I'm more focused on the upcoming 0.12.4 compiler release right now.

[drewolson]

@hdgarrood totally understood. No rush from my end, just didn't want to leave it hanging around if it was cruft. Appreciate the response!