Compatibility with transformers-0.6 and mtl-2.3 about lens

[RyanGlScott]

I've prepared an mtl-2.3 branch that should (I believe) build with transformers-0.6.* and mtl-2.3.*. I'm currently holding off on merging this branch into master since the benchmarks currently do not compile with these libraries due to simonmar/monad-par#74, which is a transitive dependency of criterion.

[RyanGlScott]

I'll work on preparing a new release with these changes soon. To avoid having to make a new major release, I'll create a 5.1 branch and backport all of the non-breaking changes that have happened since the last Hackage release. By my count, that's everything except #994 (which is definitely a breaking change) and #998 (which likely isn't a breaking change, but TH codegen changes are tricky, so I'll hold off on that just to be on the safe side).

[RyanGlScott]

I've uploaded lens-5.1.1 to Hackage and applied a Hackage revision to lens-properties-4.11.1.

[sjakobi]

If I build with mtl-2.3 and transformers < 0.6, I get compiler errors too:

src/Control/Lens/Zoom.hs:177:10: error:
    • Could not deduce (MonadState s (ListT m))
        arising from the superclasses of an instance declaration
      from the context: Zoom m n s t
        bound by the instance declaration
        at src/Control/Lens/Zoom.hs:177:10-53
    • In the instance declaration for ‘Zoom (ListT m) (ListT n) s t’
    |
177 | instance Zoom m n s t => Zoom (ListT m) (ListT n) s t where
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

src/Control/Lens/Zoom.hs:177:10: error:
    • Could not deduce (MonadState t (ListT n))
        arising from the superclasses of an instance declaration
      from the context: Zoom m n s t
        bound by the instance declaration
        at src/Control/Lens/Zoom.hs:177:10-53
    • In the instance declaration for ‘Zoom (ListT m) (ListT n) s t’
    |
177 | instance Zoom m n s t => Zoom (ListT m) (ListT n) s t where
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

src/Control/Lens/Zoom.hs:185:10: error:
    • Could not deduce (MonadState s (ErrorT e m))
        arising from the superclasses of an instance declaration
      from the context: (Error e, Zoom m n s t)
        bound by the instance declaration
        at src/Control/Lens/Zoom.hs:185:10-70
    • In the instance declaration for
        ‘Zoom (ErrorT e m) (ErrorT e n) s t’
    |
185 | instance (Error e, Zoom m n s t) => Zoom (ErrorT e m) (ErrorT e n) s t where
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

src/Control/Lens/Zoom.hs:185:10: error:
    • Could not deduce (MonadState t (ErrorT e n))
        arising from the superclasses of an instance declaration
      from the context: (Error e, Zoom m n s t)
        bound by the instance declaration
        at src/Control/Lens/Zoom.hs:185:10-70
    • In the instance declaration for
        ‘Zoom (ErrorT e m) (ErrorT e n) s t’
    |
185 | instance (Error e, Zoom m n s t) => Zoom (ErrorT e m) (ErrorT e n) s t where
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cabal: Failed to build lens-5.1

[phadej]

Just sayin: there is no patch for monad-par and I'm somewhat sure that Simon won't write it himself any time soon.

That said, statistics depends on on monad-par just to do parMap in Statistics.Resample.Bootstrap, which most people use parallel package for (EDIT: monad-par isn't used on GHCJS, see https://github.com/haskell/statistics/pull/153/files, and I tried to drop monad-par completely in https://github.com/haskell/statistics/pull/154/files, but that was a major change, maybe change to use parallel will be more welcome) . Yet, that package is also maintained by simonmar (but arguably is a simpler one, and lens depends on parallel directly as well, so maybe a better dependency overall - both parallel and monad-par is slightly silly).

[RyanGlScott]

If there's an easy way to trim down criterion's dependencies, I'm all for it. Either way, we'll need some kind of action downstream.

[phadej]

I made a PR to statistics, haskell/statistics#187

[RyanGlScott]

statistics-0.16.1.0 no longer depends on monad-par, so that is no longer a blocker.

The next blocker is haskellari/microstache#30.

[stevenfontanella]

@RyanGlScott I looked at your branch, and I was curious why the transformers constraint is needed on the Zoom instances for ErrorT and ListT (src/Control/Lens/Zoom.hs line 196)? Those types should be defined in mtl (<2.3), not transformers. I tried removing the transformers constraint and was still able to build with

cabal v2-build --allow-newer=transformers

I wanted to know since I am doing the same migration for microlens and want to make sure I get it right.

[RyanGlScott]

Are you referring referring to this code?

#if !MIN_VERSION_transformers(0,6,0) && !MIN_VERSION_mtl(2,3,0)
instance (Error e, Zoom m n s t) => Zoom (ErrorT e m) (ErrorT e n) s t where
  zoom l = ErrorT . liftM getErr . zoom (\afb -> unfocusingErr #. l (FocusingErr #. afb)) . liftM Err . runErrorT
  {-# INLINE zoom #-}

instance Zoom m n s t => Zoom (ListT m) (ListT n) s t where
  zoom l = ListT . zoom (\afb -> unfocusingOn . l (FocusingOn . afb)) . runListT
  {-# INLINE zoom #-}
#endif

The reason those version bounds are in place is because:

1. ErrorT and ListT are imported from Control.Monad.Trans.Error and Control.Monad.Trans.List, which are defined in transformers prior to version 0.6.0.0.
2. Zoom has a MonadState superclass, so in order for these instances to typecheck, the MonadState instances for ErrorT and ListT from Control.Monad.State (in mtl) need to be in scope. These instances are defined in mtl prior to version 2.3.

I'm not sure why you'd need to modify the bounds to get things building. Do you experience an error of some kind with the bounds as-is?

[stevenfontanella]

Yes that's the code. I changed the first line to

#if !MIN_VERSION_mtl(2,3,0)

and forced cabal to use transformers ==0.6 and was still able to build.

Thanks, you cleared up my confusion. I was confused because both mtl and transformers have definitions for ErrorT and ListT. In this case seems we are importing the types from transformers, not mtl as I thought.

I realized now that by forcing transformers ==0.6, it (transitively) forced mtl 2.3, because mtl 2.2.2 requires transformers <0.6, so the mtl version check is enough in that case. If you force transformers ==0.6 AND mtl ==2.2.2, then mtl will fail to build anyway (since it defines instances for ListT and ErrorT which don't exist in transformers 0.6).

So in summary, mtl >=2.3 already implies transformers >=0.6. For microlens, I will do the same version checks that you are doing to be explicit, and in case mtl lowers its transformers lower bound.

To answer your last question: I didn't need to modify any bounds, but I was just testing if the transformers bound was necessary so I pinned transformers ==0.6.

Thanks for the help!

[RyanGlScott]

So in summary, mtl >=2.3 already implies transformers >=0.6.

I don't think this is the case, since mtl-2.3's lower bounds on transformers are >= 0.5.6. As a result, it's possible to import ErrorT and ListT from transformers-0.5.6 while still not having MonadState instances defined for them in mtl-2.3.

The opposite direction—i.e., transformers >= 0.6 implying mtl >= 2.3—may very well be true. But I've been burned by having imprecise CPP before, so I've found it's best to be as explicit as possible.