Order dependence for makeFields

[jokesper]

The following code does not compile, while it would if you switch the last two lines.

{-# LANGUAGE FlexibleInstances #-}
{-# LANGUAGE FunctionalDependencies #-}
{-# LANGUAGE TemplateHaskell #-}
{-# LANGUAGE DuplicateRecordFields #-}
module Foo where

import Control.Lens.TH (makeFieldsNoPrefix)

data A a = A {_a :: a}
data B a = B {_a :: a} | C

makeFieldsNoPrefix ''A
makeFieldsNoPrefix ''B

This is due to the first call to makeFieldsNoPrefix defining the class
and it either containing a definition with type Lens' s a or Travesal' s a.

[RyanGlScott]

Right, this is a somewhat unfortunate aspect of the way makeFields is implemented. It wouldn't be straightforward to fix this, either. Due to Template Haskell staging restrictions, when GHC runs the makeFieldsNoPrefix ''A line, it has no knowledge of any code that comes after that line. As a result, GHC can't anticipate the call to makeFieldsNoPrefix ''B afterwards—if it could, it could generate a type signature with a Traversal' instead of an Iso'.

At a minimum, I think we should document this behavior. We might also consider adding a variant of makeFields that lets you write code like above. Two possibilities:

1. A variant of makeFields that lets you generate the class without any instances. This would let you write something like this (naming subject to change):

   makeFieldsClass ''B
   makeFieldsNoPrefix ''A
   makeFieldsNoPrefix ''B

   Where makeFieldsClass ''B does nothing but generate this class:

   class HasA s a | s -> a where
     a :: Traversal' s a

2. A variant of makeFields that prevent the use of Iso in the generated class. I'm imagining something like:

   makeFieldsNoPrefixForceTraversal ''A
   makeFieldsNoPrefix ''B

   This might be achievable by piggybacking on top of the _allowIsos fields of LensRules, but I haven't looked closely at it.

[jokesper]

This could be fixed by changing the generated class for HasA to:

class HasA s
  type A' :: Type
  type Kind :: Type -> Type -> Type
  a :: Kind s A'

This would require changing Functional Dependencies to Type Families
since adding another type parameter and adding it to the functional dependency would not allow us to create an instance easily:

class HasA s a kind | s -> a kind where
  a :: kind s a
instance HasA (A a) a Traversal' where
  ...

where the instance is equivalent to:

instance HasA (A a) a (forall f. Applicative f => (a -> f a) -> A a -> f (A a)) where
  ...

Which does not compile due to: Illegal polymorphic type since the correct version would be:

instance Applicative f => HasA (A a) a ((a -> f a) -> A a -> f (A a)) where
  ...

The approch with the Functional Dependencies would be a lot of work.
And the Alternative with Typefamilies would only require an extra language extension by the caller. I would suggest adding this to the docs and marking the old function as deprecated while intoducing a new version (maybe makeFields'?). This would remove unneeded restrictions on the instance for A, as it wouldn't require Applicative, only Functor.

[RyanGlScott]

This could be fixed by changing the generated class for HasA to:

class HasA s
  type A' :: Type
  type Kind :: Type -> Type -> Type
  a :: Kind s A'

I'm not enthusiastic about this idea, as it would mean that writing code that is polymorphic over a HasA constraint would now have to introduce constraints like:

f :: (HasA a, Kind s A' ~ Traversal') => ...

It's possible, but in more equality constraints than I'm usually comfortable with.

[jokesper]

Oh I never thought of this, since I just recently started using makeFields.
Also it would only be:

f :: (HasA a, Kind ~ Traversal') => ...

The other option would be to expand the Traversal' and such and move the type class contraints to
the beginning but this would probably be to much work.
A note in the docs should do the trick in almost all cases anyways.
Maybe adding it as an option without deprecating the "old" method would be nice but it would be a lot of work.

[jokesper]

I just noticed a bigger problem:
Even when switching the order, such that it compiles, I cannot use view on a for A:

getAa :: A -> a
getAa = view a

[jokesper]

I just tried to implement it and noticed that even with typefamilies the solution doesn't work as you must have the constrains outside the type. The only solution which could be implemented (I think) is expanding the type alias and extracting the restriction to outside the instance declaration, which, if possible, would be very cumbersome, both for the implementation and for functions polymorphic over HasA.

[ncaq]

My solution.

$(do
  makeFieldsNoPrefix ''A
  makeFieldsNoPrefix ''B
 )

[jokesper]

does do do something different then just writing them directly below each other?
Wouldn't that just discard the first?
I think this is more harmful then good. Since you would only get the most general optic.

[jokesper]

I just noticed a bigger problem:
Even when switching the order, such that it compiles, I cannot use view on a for A:

getAa :: A -> a
getAa = view a

Pretty much exactly this.

[ncaq]

I wrote something off the mark, sorry.
I was confusing the issue of strict cross-referencing of GHC 9 with the issue of GHC 9 cross-referencing, as I had recently solved that myself with DSL.