extra icon indicating copy to clipboard operation
extra copied to clipboard

How to avoid clash of imports with `compareLength` in `base`?

Open Bodigrim opened this issue 1 year ago • 4 comments
trafficstars

GHC HEAD has recently acquired Data.List.compareLength and Data.List.NonEmpty.compareLength:

Data.List.compareLength :: [a] -> Int -> Ordering
Data.List.NonEmpty.compareLength :: NonEmpty a -> Int -> Ordering

The former identifier will clash with Data.List.Extra.compareLength:

compareLength :: (Ord b, Num b, Foldable f) => f a -> b -> Ordering

What shall we do about it? Note that the existing helper in extra is more polymorphic than what will be provided from base. The simplest solution would be just

import Data.List hiding (compareLength)

so that Data.List.Extra sticks to the polymorphic entity.

Another, more disruptive option is to move the existing Data.List.Extra.compareLength under Data.Foldable.Extra and re-introduce a monomorphic Data.List.Extra.compareLength, which either restricts Data.Foldable.Extra.compareLength to [a] and Int or re-exports Data.List.compareLength if base is sufficiently new. This feels nicer to me, but, strictly speaking, constitutes a breaking change.

@ndmitchell what do you think?

Bodigrim avatar Jul 05 '24 22:07 Bodigrim

Either option sucks. I think adding compareLength with the Foldable constraint to Foldable makes sense, regardless. The the two options are:

  1. Make Data.List.Extra have a different type to base.
  2. Breaking change.

I don't love 2, but it seems consistency is a fundamental principle of extra, so 1 is worse? Keen to get other opinions though.

ndmitchell avatar Jul 28 '24 21:07 ndmitchell

The purpose of Num b eludes me. I can't imagine wanting to know if a Foldable container has more than, say, exactly 5.2 members. If the change to Int is done at the same time as the move to Data.Foldable.Extra, I don't think many users will experience unrecoverable breakage.

mixphix avatar Jul 30 '24 19:07 mixphix

Agree with others here.

  1. Do what the ancients did. Add compareLength to Data.Foldable and re-export that from Data.List.

  2. And drop the Num b => b. It's more trouble than it's worth. Just make it Int.

TBH, though, IDK why this function exists?

xs :: SomeFoldable A
ys :: OtherFoldable B
n :: Double

comparison :: Ordering
comparison = compare (length xs) (length ys)

comparison' :: Ordering
comparison' = compare (foldMap (\_ -> Sum 1) xs) (Sum n)

weird.

friedbrice avatar Oct 10 '24 01:10 friedbrice

compareLength can handle infinite lists. Your comparison and comparison' can't.

ChickenProp avatar Oct 10 '24 09:10 ChickenProp

Now as GHC 9.12 has been pre-released, the issue might be more pressing:

[ 7 of 22] Compiling Data.List.Extra  ( src/Data/List/Extra.hs, dist/build/Data/List/Extra.o, dist/build/Data/List/Extra.dyn_o )
src/Data/List/Extra.hs:23:34: error: [GHC-87543]
    Ambiguous occurrence ‘compareLength’.
    It could refer to
       either ‘Data.List.compareLength’,
              imported from ‘Data.List’ at src/Data/List/Extra.hs:42:1-16,
           or ‘Data.List.Extra.compareLength’,
              defined at src/Data/List/Extra.hs:912:1.
   |
23 |     drop1, dropEnd1, mconcatMap, compareLength, comparingLength,
   |                                  ^^^^^^^^^^^^^

andreasabel avatar Oct 17 '24 18:10 andreasabel

I've just released extra 1.8 which makes compareLength match GHC 9.12, and added compareLength to Foldable and NonEmpty. I switched to Int everywhere, as while there is value in having compareLength work on things like natural numbers, it's better to be like GHC. I don't actually have GHC 9.12, so would appreciate someone confirming that it works as expected.

ndmitchell avatar Oct 19 '24 13:10 ndmitchell