Conversation
972fb7d to
08a229a
Compare
tbagrel1
left a comment
There was a problem hiding this comment.
I think zipWithK is very close to a dual-fold/bi-fold/zip-fold (not sure if there is a standard name), if we fuse the f and cons functions into one of type a -> b -> r -> r. Maybe it would make for a more general/intuitive function?
| where | ||
| go :: [a] %1 -> [b] %1 -> r | ||
| go [] [] = nil | ||
| go (a : as) [] = lefta a as |
There was a problem hiding this comment.
It's not entierely clear why the left{a,b} functions are taking head and tail as separate arguments. I guess the motivation is to make a NonEmpty list in zipWith' without having to call NonEmpty.fromList?
There was a problem hiding this comment.
You could say that. Or equivalently, that lefta has all the available information. Or equivalently, that it makes invalid states unrepresentable. It's the natural type for this function don't you think?
| go [] (b : bs) = leftb b bs | ||
| go (a : as) (b : bs) = cons (f a b) (go as bs) | ||
|
|
||
| zipWith3 :: forall a b c d. (Consumable a, Consumable b, Consumable c) => (a %1 -> b %1 -> c %1 -> d) -> [a] %1 -> [b] %1 -> [c] %1 -> [d] |
There was a problem hiding this comment.
Why does zipWith3 doesn't get the same treatment with a zipWithK3?
There was a problem hiding this comment.
Because there's no zipWith3' with rests. And that, in turn, is because we didn't give it a type (using Either is a bit of a shortcut for zipWith', maybe it was a bad shortcut).
We could make a zipWith3K if we decide to export it.
| zipWithk f (:) [] consume2 consume2 | ||
| where | ||
| consume2 :: forall x y z. (Consumable x, Consumable y) => x %1 -> y %1 -> [z] | ||
| consume2 x y = x `lseq` y `lseq` [] |
There was a problem hiding this comment.
In zipWith3 you do (x, y) `lseq` [] instead, any reason to prefer one or the other?
There was a problem hiding this comment.
I don't think it makes much difference after optimisation. In principle, even without optimisation x `lseq` y `lseq` [] is going to be one fewer allocation.
aspiwack
left a comment
There was a problem hiding this comment.
I really thought I replied at the time. Got consumed by silly tasks in the meantime, let me address the comments quickly today and then merge.
| zipWithk f (:) [] consume2 consume2 | ||
| where | ||
| consume2 :: forall x y z. (Consumable x, Consumable y) => x %1 -> y %1 -> [z] | ||
| consume2 x y = x `lseq` y `lseq` [] |
There was a problem hiding this comment.
I don't think it makes much difference after optimisation. In principle, even without optimisation x `lseq` y `lseq` [] is going to be one fewer allocation.
| where | ||
| go :: [a] %1 -> [b] %1 -> r | ||
| go [] [] = nil | ||
| go (a : as) [] = lefta a as |
There was a problem hiding this comment.
You could say that. Or equivalently, that lefta has all the available information. Or equivalently, that it makes invalid states unrepresentable. It's the natural type for this function don't you think?
| go [] (b : bs) = leftb b bs | ||
| go (a : as) (b : bs) = cons (f a b) (go as bs) | ||
|
|
||
| zipWith3 :: forall a b c d. (Consumable a, Consumable b, Consumable c) => (a %1 -> b %1 -> c %1 -> d) -> [a] %1 -> [b] %1 -> [c] %1 -> [d] |
There was a problem hiding this comment.
Because there's no zipWith3' with rests. And that, in turn, is because we didn't give it a type (using Either is a bit of a shortcut for zipWith', maybe it was a bad shortcut).
We could make a zipWith3K if we decide to export it.
I'd missed this. Yes, it is a |
I was wrong, the zip-fold thing yields the very same code. Excellent idea! There is no such function in the Hoogle database at all. I'll name it |
69a4215 to
b73d0f4
Compare
|
Ok. I'll merge when green. I did export the I didn't write a I'll cut a minor release with this fix on Tuesday, so we have until then to figure out if there's a scalable way to represent leftovers wider |
b73d0f4 to
cab9d3e
Compare
|
Thanks for taking time to address my comments! LGTM |
Fixes #491