[GHC] #7913: Argument order not preserved by nubBy

[GHC]

#7913: Argument order not preserved by nubBy
---------------------------------+------------------------------------------
    Reporter:  paullik           |       Owner:              
        Type:  bug               |      Status:  new         
    Priority:  normal            |   Milestone:              
   Component:  Prelude           |     Version:  7.6.3       
    Keywords:  nubBy             |          Os:  Linux       
Architecture:  Unknown/Multiple  |     Failure:  None/Unknown
  Difficulty:  Unknown           |    Testcase:              
   Blockedby:                    |    Blocking:              
     Related:                    |  
---------------------------------+------------------------------------------
Changes (by simonpj):

  * difficulty:  => Unknown


Comment:

 OK, here is the implementation of `nubBy`, both reference model and
 actual:
 {{{
 nubBy                   :: (a -> a -> Bool) -> [a] -> [a]
 #ifdef USE_REPORT_PRELUDE
 nubBy eq []             =  []
 nubBy eq (x:xs)         =  x : nubBy eq (filter (\ y -> not (eq x y)) xs)
 #else
 nubBy eq l              = nubBy' l []
   where
     nubBy' [] _         = []
     nubBy' (y:ys) xs
        | elem_by eq y xs = nubBy' ys xs
        | otherwise       = y : nubBy' ys (y:xs)

 -- Not exported:
 -- Note that we keep the call to `eq` with arguments in the
 -- same order as in the reference implementation
 -- 'xs' is the list of things we've seen so far,
 -- 'y' is the potential new element
 elem_by :: (a -> a -> Bool) -> a -> [a] -> Bool
 elem_by _  _ []         =  False
 elem_by eq y (x:xs)     =  y `eq` x || elem_by eq y xs
 }}}
 In `nubBy'` the `xs` is the list of all earlier elements that have been
 accepted into the result list.  Indeed when we compare a later item `y`
 with these, the `y` parameter should be second, as it is in the reference
 implementation.

 So if I'm right the fix should be to swap the order of arguments to `eq`
 in the definition of `elem_by`.   And document all this more clearly in
 the defn of `nubBy`.  And if we were paranoid we might do the same thing
 for `nub`, really, just in case `(==)` isn't symmetrical.

 Does everyone agree?  Easy to do. Would someone like to make a patch?

 Simon

-- 
Ticket URL: <http://hackage.haskell.org/trac/ghc/ticket/7913#comment:1>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler