-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add Data.List.unsnoc :: [a] -> Maybe ([a], a) #165
Comments
Heartily in favour. |
Sounds like an excellent function to be added 👍🏻 If this function is so popular, it makes sense to standardize it and provide a properly-lazy implementation (not sure that all 22 packages implement it correctly). I trust you to write great documentation with examples and clear complexity explanations, so I don't worry about that 🙂 |
I don't think this function is very practical, because it's rather slow and is only lazy on one side. But I think it may still have a place for "roughing out" code before choosing a sequence type, and for introducing that as a general sequence operation. FWIW, I think we should consider adding |
A search for the more general |
+1 |
I agree with this. However, it is more practical (due to being total) than
I'm not sure what this means. |
Suppose I write oof xs = case unsnoc xs of
Nothing -> 12
Just (ys, y)
| y == 0 = sum ys
| otherwise = 15 This will build some awful structure in memory and ultimately be even worse for performance than using More subtly, how will this perform? goof xs = case unsnoc xs of
Nothing -> 17
Just (ys, y) -> sum ys * y That's up to whether the compiler decides to force |
Yes, I think the function ought to come with the caveat "make sure you finish using the first component of the result before you start using the second component". Accessing the On the other hand I still don't understand what the "awful structure" is. Can you elaborate? I don't actually fully understand Bodigrim's implementation. I can never really grok unsnoc :: [a] -> Maybe ([a], a)
unsnoc [] = Nothing
unsnoc (x:xs) = Just (unsnoc' x xs)
unsnoc' :: a -> [a] -> ([a], a)
unsnoc' x [] = ([], x)
unsnoc' x (x':xs) =
let u = unsnoc' x' xs
in (x : fst u, snd u) If we access the second component before the first then the first will end up looking like
which is unpleasant, and I'm open to persuasion that it's awful, but I don't see why yet. On the other hand, the alternative of unsnoc :: [a] -> Maybe ([a], a)
unsnoc [] = Nothing
unsnoc (x:xs) = Just (unsnoc' x xs)
unsnoc' :: a -> [a] -> ([a], a)
unsnoc' x [] = ([], x)
unsnoc' x (x':xs) =
let u = unsnoc' x' xs
xs' = fst u
in (x : xs', xs' `seq` snd u) yields a simple list (i.e. a sequence of actually evaluated
|
I was wrong; @Bodigrim's implementation won't produce any weird structure. I can't make head or tail of what yours is trying to do. |
I'd appreciate if you would give it another go. It's a direct recursive implementation and I didn't mean anything complicated by it. |
The purpose of the |
Can we get concrete evidence for such claims? I find it hard to follow this otherwise:
|
The first component returned by the first version of
The first component of the heap returned by the second version looks like
Since you were worried about the heap object constructed in memory I wondered if you'd be happier with the second version. |
No, I just made a mistake about the first. @Bodigrim's has the advantage of participating in list fusion. |
I don't see a way to sneak a LGTM already. |
@stephen-smith , no, but the |
I've updated the top post with a link to the MR (https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10442/diffs) and impact assessment (only trivial name clashes, all PRs are ready). |
After 8 years with Haskell, I finally learned while In any case, left a documentation suggestion in the PR. |
If there are no further questions / comments / suggestions over the weekend, I'll trigger a vote next week. |
(changing hats) Dear CLC members, let's vote on the proposal to add @tomjaguarpaw @chshersh @hasufell @mixphix @angerman @parsonsmatt +1 from me, unsurprisingly. |
+1 |
4 similar comments
+1 |
+1 |
+1 |
+1 |
Thanks all, 6 votes in favour are enough to approve. |
See haskell/core-libraries-committee#165 for discussion
For anyone reading up on the discussion after the fact (like me), I want to leave a few remarks. @tomjaguarpaw wrote:
The implementation in question: unsnoc :: [a] -> Maybe ([a], a)
unsnoc = foldr (\x -> Just . maybe ([], x) (\(~(a, b)) -> (x : a, b))) Nothing It's not easy to visually disentangle this definition, but the accumulator here is not a function, but just I'm going to try to simplify the code through a few (hopefully obvious) steps. Un-inline (outline?) the step function: unsnoc = foldr step Nothing
where
step x = Just . maybe ([], x) (\(~(a, b)) -> (x : a, b)) Eta-expand: unsnoc list = foldr step Nothing list
where
step x z = (Just . maybe ([], x) (\(~(a, b)) -> (x : a, b))) z Inline/expand unsnoc list = foldr step Nothing list
where
step x z = Just (maybe ([], x) (\(~(a, b)) -> (x : a, b))) z) Inline/expand unsnoc list = foldr step Nothing list
where
step x z = Just (case z of
Nothing -> ([], x)
Just (~(a, b)) -> (x : a, b)) In prose: |
Thanks! Perhaps you're missing a |
Thanks @mauke, I find your version much more readable and it's clear what's going on. |
Currently
base
does not offer any ergonomic total replacement forData.List.init :: [a] -> [a]
andData.List.last :: [a] -> a
. There is also no efficient way to computeinit
andtail
simultaneously, which is a very common task. Similar issues withhead
andtail
are resolved byuncons :: [a] -> Maybe (a, [a])
, so let's introduce its dual.The complete MR is available at https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10442/diffs
Implementation
It's important to use a lazy pattern match
~(a, b)
in the definition ofunsnoc
above. Otherwise the function becomes non-productive on infinite lists and (probably more importantly) is prone to stack overflow. Compare against a hypothetical stricter version, without~
:It's easier to spot the difference running
ghci
with artificially limited stack size:Here is another test to distinguish
unsnoc
andunsnoc'
:The difference between
unsnoc
andunsnoc'
is similar tofoldr
vs.foldr'
. See also #133, comparingbreak
tobreak'
.The laziness of the proposed
unsnoc
precisely matches the behaviour of a naive, inefficient implementation viainit
/last
:Impact assessment
Adding a new entity is not a breaking change according to PVP. GHC emits a
-Wcompat
warning for unqualified import ofData.List
. Still some packages might dismiss the warning (or never enable it), thus there is a possibility of name clashes. I've builtclc-stackage
against GHC 9.4 with the proposed patch and prepared PRs to all affected packages:Prior art
22 packages, including
Cabal-syntax
,filepath
,unix
, maintain their own implementations ofunsnoc
. I believe it is time to offer it fromData.List
.The text was updated successfully, but these errors were encountered: