Conversation
There is nothing wrong with linear functions for infinite lists. Presumably they will eventually be used in an unrestricted context. But this is not an issue
It's hard to justify linear
There is nothing wrong with partial function (as long as they have |
|
Thanks @aspiwack , I applied your suggestions, with below exception:
I still think from an user perspective it would be surprising to import a linear module and get an unrestricted function, especially when there is a (admittedly not ideal) linear implementation too. So I tried to find a middle-ground, and implemented the linear versions: head :: (HasCallStack, Consumable a) => [a] #-> a
tail :: (HasCallStack, Consumable a) => [a] #-> [a]
last :: (HasCallStack, Consumable a) => [a] #-> a
init :: (HasCallStack, Consumable a) => [a] #-> [a]
find :: Dupable a => (a #-> Bool) -> [a] #-> Maybe aEspecially I think this way things will be less surprising. Let me know if you still prefer the unrestricted versions instead of the above ones. |
This commit adds linear versions of the majority of Data.List functions. In order to do that, it also adds `Consumable` instances for `NonEmpty` and `Either`. It causes a minor interface change, from ``` foldl :: (b #-> a -> b) -> b #-> [a] -> b ``` to ``` foldl :: (b #-> a #-> b) -> b #-> [a] #-> b ``` Which is not a major issue since the former can be derived from latter by wrapping list elements with `Ur`.
9e8f2b7 to
dd0bb81
Compare
dd0bb81 to
9868fdc
Compare
These are mostly useless though. Plus, they are not compatible with the corresponding non-linear version. Same as record projections: they require unrestricted arguments. Even in a linearly typed module. I think that these functions ought to as well. We shouldn't require programmers to import two modules just for the sake of having mixed linear and non-linear code (which is the norm in linearly typed modules anyway). |
I agree.
I am not a fan of having to import multiple modules either. My omittance was mostly about providing a syntactic clue about whether something is working on linear or unrestricted values. Qualified imports are one way to achieve it, putting a suffix to the names would also work (but not pretty), or just having it absent from As you mentioned, whether a linear version of something is useful or not is a blurry line. Especially, for examples like But, it is not a huge issue, and definitely not a hill I want to die on before the release. So I removed the linear implementations and re-exported the unrestricted ones from |
d430352 to
8575e3e
Compare
Divesh-Otwani
left a comment
There was a problem hiding this comment.
Nice and clean code. Please make an issue to eventually remove the gratuitous uses of Unsafe in this module. We should only use Unsafe when there's a non-linear optimized implementation already in base that we want to copy.
| , unzip3 | ||
| ) where | ||
|
|
||
| import qualified Unsafe.Linear as Unsafe |
There was a problem hiding this comment.
No.......... I had hoped this wasn't going to be here. But, it might all work out 🤞
There was a problem hiding this comment.
I agree that currently this module isn't look very safe since we can not ensure that none of those unsafe casts actually drop/duplicate values.
However, Data.List has a lot of tricks to make things fast; and implementing them here would be more work. I think this matches your sentence here:
We should only use Unsafe when there's a non-linear optimized implementation already in base that we want to copy.
(Also, it is likely that the unsafeCoerce's there probably get in the way of many optimisations so I do not think this will be as fast as the Data.List even in this shape).
There was a problem hiding this comment.
I might have looked at the wrong files but when I looked at what's underneath Data.List for some of the coerced functions I didn't find optimized versions. In any case, not something we need to worry about right now. I'm not worried about safety, since most of these look like they could have linear implementations. Still, we should have an issue to double check.
There was a problem hiding this comment.
Oh yes, I did not make that choice per-function basis. I just looked Data.List overall, saw some gnarly bits and decided to implement this module by coerces. You are probably right that some of the functions would be easy to duplicate here. I'll create an issue.
| leftovers `lseq` ret | ||
|
|
||
| -- | Same as 'zipWith', but returns the leftovers instead of consuming them. | ||
| zipWith' :: (a#->b#->c) -> [a] #-> [b] #-> ([c], Maybe (Either (NonEmpty a) (NonEmpty b))) |
Closes #170, discussion at #126 .
This PR adds linear versions of the majority of Data.List and Data.Foldable functions, specializing them to work on lists. Most of them are implemented by unsafe coerces, and the ones requiring
ConsumableorDupabletypeclasses reimplemented. I hope I got the casts right (meaning, they aren't omitting or duplicating any elements without using the required typeclasses).I also omitted a few methods from
Data.List, if theyiterate,repeat.head,find,null(or they require significant changes on the type signatures)scan1,foldl1The latter two points are arbitrary decisions on my part, so let me know if you disagree and I can add them back. In my opinion, for those cases the user might find manually implementing them tailored to they needs easier. And we can always add them in future if someone requests them, or when we decide to.
It introduces a minor interface change:
Which, I think is not a major issue since the former can be derived from latter by wrapping list elements with
Ur. I had to do this for one of our examples.Similar to #174, it only copies over minimal amount of documentation from
Data.List, to reduce the amount of duplication. Let me know if you prefer otherwise.It also:
forgettoPrelude.Linear.Internalto break an import cycle.Consumableinstances forNonEmptyandEither, since they were necessary to reduce some duplication on zipping functions.Control.Monad.Linear.foldMwith explicit recursion instead offoldrto break an import cycle (because nowfoldris imported fromData.List.Linearinstead ofPrelude.Linear.Internal).