Improve some Foldable methods for NonEmpty

* `length` is improved by using the default definition,
  while `foldr1` is improved by using a custom one.

* Several methods had useless lazy pattern matches
  (i.e., the functions were actually strict in those arguments).
  Remove `~`s to clarify.

Reviewers: hvr, bgamari, mpickering, nomeata

Reviewed By: bgamari

Subscribers: ygale, rwbarton, thomie, carter

GHC Trac Issues: #15131

Differential Revision: https://phabricator.haskell.org/D4677

1 files changed, 19 insertions, 3 deletions

diff --git a/libraries/base/Data/Foldable.hs b/libraries/base/Data/Foldable.hs
index 3fa57485d6..573d4c827a 100644
--- a/libraries/base/Data/Foldable.hs
+++ b/libraries/base/Data/Foldable.hs
@@ -299,11 +299,27 @@ instance Foldable [] where
 -- | @since 4.9.0.0
 instance Foldable NonEmpty where
   foldr f z ~(a :| as) = f a (List.foldr f z as)
-  foldl f z ~(a :| as) = List.foldl f (f z a) as
-  foldl1 f ~(a :| as) = List.foldl f a as
+  foldl f z (a :| as) = List.foldl f (f z a) as
+  foldl1 f (a :| as) = List.foldl f a as
+
+  -- GHC isn't clever enough to transform the default definition
+  -- into anything like this, so we'd end up shuffling a bunch of
+  -- Maybes around.
+  foldr1 f (p :| ps) = foldr go id ps p
+    where
+      go x r prev = f prev (r x)
+
+  -- We used to say
+  --
+  --   length (_ :| as) = 1 + length as
+  --
+  -- but the default definition is better, counting from 1.
+  --
+  -- The default definition also works great for null and foldl'.
+  -- As usual for cons lists, foldr' is basically hopeless.
+
   foldMap f ~(a :| as) = f a `mappend` foldMap f as
   fold ~(m :| ms) = m `mappend` fold ms
-  length (_ :| as) = 1 + List.length as
   toList ~(a :| as) = a : as
 
 -- | @since 4.7.0.0