ekmett/semigroupoids

Re-export `Foldable1` and `Bifoldable1` from `base-4.18` (GHC 9.6)

Closed this issue · 41 comments

base-4.18.0.0 (bundled with GHC 9.6) implements this CLC proposal (see this GHC commit), which adds semigroupoids' Foldable1 and Bifoldable1 classes to base. We should re-export these classes when building with a sufficiently new version of base, and provide backported versions of these classes when building with older versions of base. Note that the API of these classes in base have changed somewhat from their current form in semigroupoids, so we will need to make some API changes on the semigroupoids end.

After doing this, we can close the following issues, as they are now base issues rather than semigroupoids issues:

It would also resolve some parts of #26, but there are certainly other parts of semigroupoids to bikeshed besides Foldable1 and Bifoldable1.

I think the plan was to release https://github.com/phadej/foldable1 as a compatibility shim, then make semigroupoids to depend on base or foldable1 depending on base version. CC @phadej.

I'll be happy if someone takes over foldable1, maybe haskell-compat organization. CLC should probably coordinate with M Farkas-Dyck whether the name can still be donated, or whether the (small) compat package has to be released under different name.

EDIT: I guess it can, as foldable1 on Hackage doesn't have any maintainers: https://hackage.haskell.org/package/foldable1/maintainers/ (which is weird!?)

I don't have a strong opinion on whether to put the compatibility shims into semigroupoids or into a separate package like foldable1. That being said, I'm very unclear on what the process would be for taking over an already-released-to-Hackage library such as foldable1, which gives me pause.

That being said, I'm very unclear on what the process would be for taking over an already-released-to-Hackage library such as foldable1, which gives me pause.

I'd say that's something CLC should arrange together with Hackage admins.

@strake donated the name for the cause in 2019: https://mail.haskell.org/pipermail/libraries/2019-October/030029.html

@Bodigrim asked @strake to confirm whether that's offer still stands in 2021 in haskell/core-libraries-committee#9 (comment) but AFAICS didn't got a confirmation (nor prohibition!)

But as said, foldable1 doesn't have a maintainer anymore (which is weird). I think hackage admins can see an audit log showing when (and who) modified the maintainers group of foldable1. Also foldable1 is marked as deprecated.

So I don't see a road block for taking over the package name. One can of course be polite and ask once more, but OTOH if @strake removed themselves from a maintainer group and deprecated the package, I'd say that the offer made in 2019 still stands.

It would be better to have a separate shim package so that users who are interested in Foldable1, but do not want to depend on semigroupoids could do so without compromising GHC support range.

I assume that @strake deprecated the package and stepped down as a maintainer (leaving the package without any active maintainers), so it should be fine to take over. If someone is serious about maintaining a compatibility package, I'd be happy to assist asking Matthew again and helping with admin matters.

I think https://github.com/phadej/foldable1 would be a good candidate for a shim package. @phadej, would you be willing to transfer ownership of that repository to the haskell-compat organization?

@Bodigrim, is the package takeover process described here up to date? If so, I can initiate it by emailing @strake.

@Bodigrim, is the package takeover process described here up to date? If so, I can initiate it by emailing @strake.

Yes, it's up to date; thanks.

@phadej, would you be willing to transfer ownership of that repository to the haskell-compat organization?

Sure.

Thanks, @phadej! Let me know if I can help with anything transfer-wise.

@RyanGlScott I'll first transfer it to you, as IIRC one needs to be an admin of org to transfer the repositories there.

@RyanGlScott and also it might be required to move Bifoldable1 to bifunctors as that class depends on Bifoldable and you might end up with a package dependency circle. At least I'd prefer foldable1 not to depend on bifunctors (and stuff bifunctors depends).

(Say if indexed-traversable would like to introduce Foldable1WithIndex.)

A very good point. I can see how bifunctors' current set of dependencies would be problematic. Moving Bifoldable1 to the bifunctors library would avoid this issue for anyone who only wants to use the Foldable1 class, but the issue would still be there for those who want to use the Bifoldable1 class. This means that we still run into the same issues that @Bodigrim is getting at in #130 (comment), albeit in slightly more limited fashion.

Really, I think the issue is that the bifunctors library is trying to be a compatibility package, but it does a poor job of doing so. In addition to providing backported versions of Bifunctor, Bifoldable, and Bitraversable, the bifunctors library also provides a wide variety of extra bells and whistles that do not have anything to do with compatibility purposes. These include:

  • Tannen (which brings in a comonad dependency)
  • TH derivation (which brings in a template-haskell and th-abstraction dependency)

If you were to strip away all of this extra functionality, however, then bifunctors' dependencies would be the same as foldable1's. To me, this suggests that there is a smaller package lurking beneath the surface. I propose that we:

  • Factor out the Data.Bifunctor, Data.Bifoldable, and Data.Bitraversable compatibility modules from bifunctors and put them in their own package. I'll tentatively call this package bifunctor-classes-compat.
  • Update bifunctors to depend on (and re-export the modules from) bifunctor-classes-compat.
  • Have foldable1 depend on bifunctor-classes-compat.

Does this sound reasonable? If we wanted to, we could even rename the foldable1 library to foldable1-classes-compat for symmetry with bifunctor-classes-compat. This would have a secondary advantage: we could upload it to Hackage without needing to take over the existing foldable1 library on Hackage.

That's an option too.

For some reason I'd still rather put Bifoldable1 into bifunctor-classes-compat, but I guess it doesn't really make a difference.

EDIT: A benefit of having Bifoldable1 in foldable1-classes-compat is that bifunctor-classes-compat would be very much unused (as lower bounds for many stuff is starting to be over GHC-8.2+)

I favor putting Bifoldable1 in foldable1-classes-compat, since:

  1. As you mentioned in your edit to #130 (comment), foldable1-classes-compat will only incur an extra bifunctor-classes-compat dependency if building with GHC 8.0 or older, so recent GHCs (the more common case) won't incur any extra dependencies.
  2. I like the idea of putting Foldable1 and Bifoldable1 together into the same compatibility package, since they were also introduced into base at the same time.

I was thinking what happens if e.g. Biapplicative migrates to base as well. I guess nothing drastic.

  • the pain of that migration will be in bifunctors / bifunctor-classes-compat (whether the class will be moved or not).
  • foldable1-classes-compat won't need to change as it doesn't care where is Biapplicative, as it doesn't use it.

There might be very distant, "what if Bitraversable1 migrates to base", but it's so distant (and may never happen) that it may not be our problem to think about (and e.g. foldable1 or atleast bifunctor-classes-compat may be obsolete by then :) )

Indeed, classes migrating from the Kmettiverse to base is a pretty uncommon occurrence. I'm inclined not to worry about this for now.

I've started an attempt to split out bifunctors' classes into a separate bifunctor-classes-compat library. This almost works, save for one annoying complication. In bifunctors' Data.Bifunctor.TH.Internal module, we have code like the following:

-- By manually generating these names we avoid needing to use the
-- TemplateHaskell language extension when compiling the bifunctors library.
-- This allows the library to be used in stage1 cross-compilers.

bifunctorsPackageKey :: String
#ifdef CURRENT_PACKAGE_KEY
bifunctorsPackageKey = CURRENT_PACKAGE_KEY
#else
bifunctorsPackageKey = "bifunctors-" ++ showVersion version
#endif

mkBifunctorsName_tc :: String -> String -> Name
mkBifunctorsName_tc = mkNameG_tc bifunctorsPackageKey

mkBifunctorsName_v :: String -> String -> Name
mkBifunctorsName_v = mkNameG_v bifunctorsPackageKey

bimapConstValName :: Name
bimapConstValName = mkBifunctorsName_v "Data.Bifunctor.TH.Internal" "bimapConst"

bifoldrConstValName :: Name
bifoldrConstValName = mkBifunctorsName_v "Data.Bifunctor.TH.Internal" "bifoldrConst"

-- ...

On modern GHCs, none of this is necessary, as you can simply use the TemplateHaskellQuotes extension. On older GHCs without access to TemplateHaskellQuotes, however, we have to resort to a trick to be able to reference the appropriate Template Haskell Names for things without having to enable the TemplateHaskell extension (and thereby break cross-compilation).

There is a catch to this trick, however: it only works for things defined either in (1) the base library, or (2) the current library being built. After moving the classes to bifunctor-classes-compat, however, the classes are no longer defined in the same library as Data.Bifunctor.TH. As a result, we no longer have a way to gain access to the Names using this trick!

I'm a bit stumped at how best to handle this. Some options include:

  1. Move Data.Bifunctor.TH to bifunctor-classes-compat. This would put everything back into the same library and make the trick above possible once more. On the other hand, it would make bifunctor-classes-compat incur a th-abstraction dependency, which seems strange.
  2. Add a module to bifunctor-classes-compat that exports its version information (including the package hash) so that Data.Bifunctor.TH can make use of it. Again, this is a strange thing to export, but I don't see a better way to gain access to this information.

Sadly haskell/cabal#7027 haven't got implemented, now it would been handy

I don't think having Data.Bifunctor.TH in bifunctor-classes-compat is that bad. th-abstraction depends only on ghc boot packages, so it's not causing extra dependency problems.

Unfortunate, but could perfectly live with.

I was wondering. Is it actually true that template-haskell doesn't have a means to lookup a complete modulename (i.e. with unit-id as pkgname) using just a (short) pkgname (like used by PackageImports) and module name?

I wonder if i should open a GHC issue to resolve that eventually

I don't think having Data.Bifunctor.TH in bifunctor-classes-compat is that bad.

Sounds good to me. I'll go with that plan, then.

Is it actually true that template-haskell doesn't have a means to lookup a complete modulename (i.e. with unit-id as pkgname) using just a (short) pkgname (like used by PackageImports) and module name?

Not that I am aware of. The only ways I know to look this information up are:

  1. Pattern-matching on a Name produced with TemplateHaskellQuotes,
  2. Using the packageName function from GHC.Generics, or
  3. Using the CURRENT_PACKAGE_KEY trick (which only works for identifiers in the current package)

If I understand what you're asking, none of these would satisfy your criteria.

Yeah, it's not great.

Of course one could lookupName, but that requires the name to be in scope where the splice is run, though I don't think it will be impossible to require say Bifunctor be in scope then.

But OTOH threading the pkgname through all of the TH code is extra work...

I've moved Data.Bifunctor.TH to a draft version of bifunctor-classes-compat. Unfortunately, I've discovered a more serious issue: you can't combine cabal's reexported-modules feature with pre-7.10 versions of GHC, as seen here:

Configuring library for bifunctors-5.5.14..
cabal-3.6.2.0: Your compiler does not support module re-exports. To use this feature you must use GHC 7.9 or later.

Sigh.

I'm not sure how "serious" it is. It might act as a (welcomed, if you ask me) trigger for folks to drop GHC-7 support if they still do.

EDIT: It's an option to drop GHC-7 support from bifunctors (not necessarily bifunctor-classes-compat) e.g. as well. It is already on the roadmap (if you look at the main branch)...

Indeed, I think we will need to make some sort of backwards-incompatible change to the bifunctors library. The question is: how far should we go? Some possible options:

  1. Make the bifunctors library continue to support GHCs all the way back to 7.0, but do not re-export Data.Bifunctor and friends unless building with GHC 7.10 or later. This provides maximum compatibility at the expense of having a conditional API.
  2. Make the bifunctors library continue to support GHCs all the way back to 7.0, but do not re-export anything from bifunctor-classes-compat. This is uniformly backwards-incompatible change (notably, all users of Data.Bifunctor.TH will have to change their dependency on bifunctors to bifunctor-classes-compat), but avoids a conditional API.
  3. Drop pre-8.0 support in bifunctors, and then re-export everything from bifunctor-classes-compat. This is another way to avoid a conditional API, but at the expense of dropping some support for old GHC versions.

If I understood #130 (comment) correctly, you support option (3). There is definitely a nice consistency to that option, even though we'd have to cut off some GHC support in bifunctors to make it happen. I'll give this some thought.

FWIW I’d suggest to drop GHC < 8 unconditionally and potentially independently of other developments. Pretty much everyone else, even boot libraries, did so.

Very well, I will go with option (3) then. I've opened ekmett/bifunctors#110 to track dropping pre-8.0 support in bifunctors.

One nice benefit of option (3), which I forgot to list in #130 (comment), is that we can now use TemplateHaskellQuotes unconditionally. As a result, we would no longer need to put Data.Bifunctor.TH in bifunctor-classes-compat to work around technical limitations of pre-8.0 GHCs. Nice!

TemplateHaskellQuotes + Data.Bifunctor.TH point is very good reason to go with (3). Nice observation.

To clarify, bifunctor-classes-compat could support GHC-7 still, even in (3) way. (E.g. if it's desired for base-compat-batteries to still support GHC-7, which would need bifunctor-classes-compat to do so as well).

And another observation. base-4.9.0.0 has Data.Bifunctor etc. So the bifunctor-classes-compat makes sense only for older GHCs to begin with.

To clarify, bifunctor-classes-compat could support GHC-7 still, even in (3) way.

Yes, I'm definitely planning to do just that.

And another observation. base-4.9.0.0 has Data.Bifunctor etc. So the bifunctor-classes-compat makes sense only for older GHCs to begin with.

Not quite. Bifunctor was introduced in base-4.8.0.0, but Bifoldable and Bitraversable were introduced in base-4.10.0.0. As a result, bifunctors would still need to re-export Data.Bifoldable and Data.Bitraversable from bifunctor-classes-compat when built with base-4.9.0.0.

See ekmett/bifunctors#111 for a patch that drops pre-8.0 support in bifunctors.

And also see:

I've made some more progress on this:

  • The foldable1-classes-compat library is now essentially finished, and we could release it to Hackage now if we wanted.
  • I have opened PRs to bifunctors and tagged (here and here, respectively) to migrate over the Foldable1/Bifoldable1 instances that were defined in semigroupoids.

While prototyping a patch for semigroupoids itself, I realized that we face a design choice that we will need to make a decision about. Currently, semigroupoids exports Foldable1 and Bifoldable1 from the Data.Semigroup.Foldable module, along with various utility functions:

module Data.Semigroup.Foldable
( Foldable1(..)
, intercalate1
, intercalateMap1
, traverse1_
, for1_
, sequenceA1_
, foldMapDefault1
, asum1
, foldrM1
, foldlM1
) where

The tricky part is that there are several differences between the API exported from Data.Semigroup.Foldable and the API that Data.Foldable1/Data.Bifoldable1 in base offer:

  • The largest difference is that the base versions of the type classes offer many more class methods (e.g., foldrMap1) than what the semigroupoids versions of the classes have.
  • On the flip side, semigroupoids also defines several functions that base does not offer (e.g., asum1).

Should we make an effort to make the semigroupoids API match that of base? Should we stick to semigroupoids' existing API? Somewhere in between?

We can think that Data.Foldable1 in base supersedes Data.Semigroup.Foldable.Class in semigroupoids. The Data.Semigroup.Foldable may have more methods.

E.g. asum1 is defined using Alt (which is not Alternative), a class defined in semigroupoids. foldMapDefault1 is just foldMap1 (as base doesn't need wrap the Monoid). for1_, traverse1_ are also defined using Apply, which is defined in semigroupoids.

I'd

  • Leave Data.Semigroup.Foldable.Class as is, exporting only methods which it defines
  • Deprecate Data.Semigroup.Foldable.Class
  • Data.Semigroup.Foldable could export all from Data.Foldable1, or just what it does now, or something in between (E.g. head, tai, i.e. clashing names may be left out)

Another thing I've realized is that if we want to depend on a sufficiently new version of bifunctors that provides Foldable1 instances, then we'll no longer be able to build semigroupoids with pre-8.0 versions of GHC. We might as well drop support for them before going through with the remaining changes.

#131 drops pre-8.0 GHC support.

#132 is a draft PR that migrates semigroupoids over to using Foldable1/Bifoldable1 from base. Feedback is very much appreciated!

GHC 9.6.1-alpha3 is now out, and I believe I've put foldable1-classes-compat into a state where it is ready for a Hackage release. I'll do so within the coming days, unless there's something I've overlooked. (If I have, please speak up!)

I've uploaded semigroupoids-6 to Hackage with these changes.