Skip to content

Add deriving via Generically/Generically1 #316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 21 commits into from

Conversation

sjoerdvisscher
Copy link
Member

I think the Traversable instances nicely show the difference between traversing with linear control Applicatives and linear data Applicatives. (I'm planning to write a blogpost about this part.)

The linear control Functor generics are a bit more complicated, since you need to prove that the mapping function is used exactly once, which means having Par1 exactly once. My approach has some runtime overhead, maybe there's a better way?

@aspiwack
Copy link
Member

I'm not really very comfortable with this, until Generic actually exposes linear functions. Maybe we could have deriving via stuff, so that we can hide this capability behind a module named OmgDangerWillRobinsonHereBeDragons.Generic. Because it probably mostly works. But is not technically legal.

Also, one quick question: why does Control.Functor require a GFunctor class, but in Data.Functor you seem to reuse the Functor class on the representation?

@sjoerdvisscher
Copy link
Member Author

Using deriving via is actually a really good idea, I don't like the default signatures way.

Control.Functor needs a separate class because I need to prove that the mapping function is used only once. This doesn't work f.e. for Control.Functor:

instance (Functor f, Functor g) => Functor (f :*: g) where
  fmap f (l :*: r) = fmap f l :*: fmap f r

because f needs to be used linearly. GFunctor threads the f through, starting out as One (a %1-> b) and switching to Zero (a %1-> b) once it is used.

Par1 is where this happens:

instance GFunctor One Zero Par1 where
  gmap (One f) (Par1 a) = (Par1 (f a), Zero)

And for the product instance you see the threading happening:

instance (GFunctor i m l, GFunctor m o r) => GFunctor i o (l :*: r) where
  gmap f (l :*: r) = gmap @i @m f l & \case
    (l1, f') -> gmap @m @o f' r & \case
      (r1, f'') -> (l1 :*: r1, f'')

@aspiwack
Copy link
Member

Makes sense. I was trying to remember why I've always needed a separate class for the generic thing. I haven't made a generic deriver for quite a while. And maybe it actually was for this very reason of needing extra parameters.

@sjoerdvisscher
Copy link
Member Author

You also need a separate class for classes of kind Type -> Constraint, reusing the same class only works for classes of kind (Type -> Type) -> Constraint because GHC.Generics gives everything this p argument.

@sjoerdvisscher sjoerdvisscher changed the title Derive Functor and Traversable instances with GHC.Generics Add deriving via Generically/Generically1 Mar 2, 2021
Copy link
Member

@aspiwack aspiwack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve of the new implementation strategy for deriving control functors. But it needs a little documentation for when we come back there later 🙂

Aside from that (and the comments below), may I ask of you to add an example file in the examples/ directory. Probably with a couple of tests (we have a dedicated test suite for the examples).

import Prelude.Linear.Internal
import Data.V.Linear ()

newtype Generically a = Generically a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some Haddock to explain why one would use Generically, and why it is presently unsafe. And when it is safe to use.

eitherNoPar1Map f (l :*: r) = unused l :*: fmap f r
instance (Unused r, Functor l, NoPar1 l ~ 'False) => EitherNoPar1 'False 'True l r where
eitherNoPar1Map f (l :*: r) = fmap f l :*: unused r
type MessageMany = 'Text "Can't derive an instance of Functor. One of the constructors of your datatype uses the type parameter more than once."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine for the first brush. But I think that we can improve on this error message by carrying the name and type of the offending constructor and adding it to the error message.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aspiwack, yes, we'll want to add a GFunctor class that takes the name of the type as well as the constructor.

@aspiwack
Copy link
Member

Thanks @treeowl for raising this again

I remember that there was an issue with this Pull Request which prevented us from moving forward. But it seems that we failed to document the problem here. @sjoerdvisscher do you remember what happened?

@aspiwack aspiwack mentioned this pull request Sep 29, 2021
@sjoerdvisscher
Copy link
Member Author

The problem is that the generated code by deriving Generic1 for types having :.: in the representation uses regular fmap, but it needs to be linear for our use cases. I guess this is also what @treeowl found out.

@treeowl
Copy link
Collaborator

treeowl commented Sep 29, 2021

The problem is that the generated code by deriving Generic1 for types having :.: in the representation uses regular fmap, but it needs to be linear for our use cases. I guess this is also what @treeowl found out.

I wasn't even thinking about that aspect. Where does fmap show up in generated code? I remember it shows up in a lot of Generic1-based instances. The fundamental error in Generic1 deriving is the way it constructs compositions. It should associate them to the left (e.g., (Rec1 (Either Int) :.: []) :.: Maybe), but instead associates them to the right (Either Int :.: ([] :.: Rec1 Maybe)). Is it possible to patch this up for linear-base?

@sjoerdvisscher
Copy link
Member Author

sjoerdvisscher commented Sep 29, 2021

I remember it shows up in a lot of Generic1-based instances.

Yes that's what I meant, the generated instances.

I still don't know what the best way is to do deriving of linear generics. I was hoping to do it with some type level hackery and maybe a minimal amount of TH based on the generated instances of regular generics, but I couldn't really figure that one out. Doing everything in TH seems a daunting task, maybe there's a library that we can hook into? Or else baking it in into GHC might be the easiest approach?

@treeowl
Copy link
Collaborator

treeowl commented Sep 29, 2021

Generic1 isn't nearly as popular as Generic, so I think it's best to start with the latter. But I'll make an attempt at a hacked Generic1.

@aspiwack
Copy link
Member

Generic1 isn't nearly as popular as Generic, so I think it's best to start with the latter. But I'll make an attempt at a hacked Generic1.

True, but this is partly because there are stock derivers for Functor/Foldable/Traversable. Maybe we ought to split the PR to get the easy bits in, though.

@treeowl
Copy link
Collaborator

treeowl commented Sep 29, 2021

Where are the linear to and from exported? I couldn't figure that out easily from the PR. Yes, I think splitting is a great idea.

@treeowl
Copy link
Collaborator

treeowl commented Oct 12, 2021

Could you possibly rebase this? I can't figure out which pieces are supposed to go where.

@treeowl
Copy link
Collaborator

treeowl commented Oct 12, 2021

I was thinking to rework this whole thing to use linear-generics, but if you have other plans I don't want to get in the way.

@sjoerdvisscher
Copy link
Member Author

@treeowl No please, go ahead, I'm done now. I was just trying to get this in the shape that's hopefully the most useful to you.

@treeowl
Copy link
Collaborator

treeowl commented Oct 12, 2021

Great, as long as we're on the same page.

@aspiwack
Copy link
Member

@treeowl note that now that you have write access you can work directly on this branch, if it's convenient to you.

@treeowl
Copy link
Collaborator

treeowl commented Oct 12, 2021

Sounds good. I'll let y'all know when I'm ready for review. I expect I'll need other people to tell me what to name everything and where to put it, since I don't know your conventions, but that can happen later.

* Switch to `linear-generics`.

* Make `Unused (f :.: g)` handle the case that `g` is `Unused`
  but `f` is not.

* Avoid `error` when facing unsatisfiable constraints.
The contents of `Unsafe.Linear` were moved to
`Unsafe.Linear.Internal`, presumably to avoid cyclical imports
with `Generically` stuff. `Generically` is no longer unsafe,
so we don't need that, at least for now. We can get it back
if we want/need, but it doesn't belong in this PR and it makes
the diff hard for me to use.
* Put the 'Generically' and 'Generically1' types in a module,
  `Prelude.Linear.Generically`, at the very base of the module
  hierarchy. This seems to help avoid the module cycle problems that
  have been worked around various ways.

* Move the generic-deriving machinery for each class into the module
  that defines that class.

* Start unwinding orphan instance spaghetti. I'm optimistic this
  will work, but I know it's not a sure thing.
The knots are coming undone. If this works, I'll eventually need to write a
proper dependency explanation, like the one in `GHC.Base`.
* Many more orphans removed.

* Many more instances added.

* Added generic deriving for data `Applicative`s.
* Improve generic `Traversable`. It works well for product types
  and sufficiently simple sum types. For nontrivial sum types the
  GHC optimizer makes hash of it in 9.0; I don't have 9.2 yet,
  so I don't know if that's changed any.

* Add `Yoneda` and `Coyoneda` for both data functors and control
  functors, and `Curried` for control functors. `Yoneda` and
  `Coyoneda` especially tend to come in handy.
@treeowl treeowl force-pushed the sv/functor-traversable-generics branch from 03e1f0b to 4415181 Compare October 14, 2021 02:34
Make `GTraversable` lazier for `V1` to match behavior typical in
`base`. Follow Sjoerd Visscher's advice adding more
`Unsatisfiable` constraints for bogus `GApplicative` instances.
@aspiwack
Copy link
Member

This branch is slated for the v2.0 release (next week-ish). @treeowl will take care of updating it to master (when he recovers from his cold). Thanks @treeowl .

@treeowl
Copy link
Collaborator

treeowl commented Feb 22, 2022

I finally felt well enough today to start the rebasing process. No major roadblocks yet. What Trustworthy module should I re-export GHC.Types.Multiplicity from so I can use it in (low dependency order) Safe modules?

@treeowl
Copy link
Collaborator

treeowl commented Feb 22, 2022

@aspiwack I'd really like to merge Data.Unrestricted.Linear.Internal.Dupable with Data.Unrestricted.Linear.Internal.Movable to get rid of orphan instances. Is that okay with you? My manual "rebase" has now reached the tricky bit (the generic Dupable stuff all needs to be redone), and I'd love to finish it up today or this weekend at the latest.

Another question: do we want Data.Functor and Traversable instances for Data.Vector.Vector and Data.Primitive.Array? I was a bit surprised not to see those.

@treeowl treeowl mentioned this pull request Feb 23, 2022
@treeowl
Copy link
Collaborator

treeowl commented Feb 23, 2022

Progress continues. I may not need to merge the Dupable and Movable modules after all. AsMovable doesn't have any obvious benefits over generic deriving for basic instances.

One frustrating thing: generic deriving of Dupable a => Dupable [a] doesn't work out all that great. The Core is quite nice, with one glaring exception: the whole dupR definition becomes a loop breaker and GHC doesn't produce an unfolding. Writing an instance manually, I can use an explicit go function with scoped type variables to get what I want. General suggestions welcomed. See #394 for the work in progress.

@aspiwack
Copy link
Member

I finally felt well enough today to start the rebasing process. No major roadblocks yet. What Trustworthy module should I re-export GHC.Types.Multiplicity from so I can use it in (low dependency order) Safe modules?

I've never payed much attention Safe Haskell, to be honest I don't really know how this works (plus linear-base has a fairly large kernel of trust).

I suggest that we don't care about it for the sake of this PR. And if you want to make linear-base safe-haskell compatible, we discuss how in a second PR.

@aspiwack I'd really like to merge Data.Unrestricted.Linear.Internal.Dupable with Data.Unrestricted.Linear.Internal.Movable to get rid of orphan instances. Is that okay with you?

I honestly don't care either way. If it's useful to your PR, feel free. If it's orthogonal, I prefer we do that in a separate step.

My manual "rebase" has now reached the tricky bit (the generic Dupable stuff all needs to be redone),

Is it worth splitting this to another PR? (I feel my remarks, so far, have been having a theme 🙂; at the end of the day, do whatever is more expedient).

Another question: do we want Data.Functor and Traversable instances for Data.Vector.Vector and Data.Primitive.Array? I was a bit surprised not to see those.

I'd say yes. It's an oversight that we haven't (but, you guessed it: it should be a different PR)

One frustrating thing: generic deriving of Dupable a => Dupable [a] doesn't work out all that great. The Core is quite nice, with one glaring exception: the whole dupR definition becomes a loop breaker and GHC doesn't produce an unfolding. Writing an instance manually, I can use an explicit go function with scoped type variables to get what I want. General suggestions welcomed.

I suggest that we keep the instance for [a] manual for the time being, and that we think of ways to improve code generation for this case when the Generic work is merged and we have mental room to focus on the problem.

@tbagrel1 tbagrel1 removed this from the v0.2.0 milestone Feb 25, 2022
Comment on lines +9 to +10
newtype Generically a = Generically a

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generically and Generically1 have been added to base-4.17. I don't know whether this is relevant for you, but I thought I'd mention it just in case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's definitely relevant. I'm not sure how we will handle this, but when 9.4 is out, we will strive to make these newtypes re-exports from base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants