Skip to content

Turn Join into a class for neater type signatures#399

Merged
arybczak merged 1 commit intomasterfrom
join-kinds
Feb 15, 2021
Merged

Turn Join into a class for neater type signatures#399
arybczak merged 1 commit intomasterfrom
join-kinds

Conversation

@arybczak
Copy link
Copy Markdown
Collaborator

@arybczak arybczak commented Feb 8, 2021

Somewhat related to https://gitlab.haskell.org/ghc/ghc/-/issues/19336 (it's a GHC bug, but it pushed me to find a solution for using a class for Join that works).

I wanted to do that for some time now, considering that the type signature of % was a bit messy, not to mention type signatures of generic optic functions (see e.g. #368). Now looks like a good time to do that since Append was also changed.

Now the signature of % is quite neat:

>>> :t (%)
(%)
  :: (JoinKinds k l m, AppendIndices is js ks) =>
     Optic k is s t u v -> Optic l js u v a b -> Optic m ks s t a b

Also, now label compositions are somewhat readable:

>>> :t #pets % traversed % #_GoldFish % #name
#pets % traversed % #_GoldFish % #name
  :: (Traversable t1, JoinKinds k1 l1 m, JoinKinds k2 l2 k1,
      JoinKinds k3 A_Traversal k2, LabelOptic "_GoldFish" l2 u1 v1 u2 v2,
      LabelOptic "name" l1 u2 v2 a b,
      LabelOptic "pets" k3 s t2 (t1 u1) (t1 v1)) =>
     Optic m NoIx s t2 a b

instead of this monstrosity

>>> :t #pets % traversed % #_GoldFish % #name
#pets % traversed % #_GoldFish % #name
  :: (Traversable t1,
      Is
        (Join (Join k A_Traversal) l1)
        (Join (Join (Join k A_Traversal) l1) l2),
      Is l2 (Join (Join (Join k A_Traversal) l1) l2),
      Is (Join k A_Traversal) (Join (Join k A_Traversal) l1),
      Is l1 (Join (Join k A_Traversal) l1), Is k (Join k A_Traversal),
      Is A_Traversal (Join k A_Traversal),
      LabelOptic "_GoldFish" l1 u1 v1 u2 v2,
      LabelOptic "name" l2 u2 v2 a b,
      LabelOptic "pets" k s t2 (t1 u1) (t1 v1)) =>
     Optic (Join (Join (Join k A_Traversal) l1) l2) NoIx s t2 a b

See also change to label6setter in code, no more weird Is constraints.

If no one protests, I'll adjust codegen.

@arybczak arybczak force-pushed the join-kinds branch 3 times, most recently from bd8608c to 32f0c27 Compare February 8, 2021 22:05
Optic k % Optic l = Optic m
where
km :: forall p i. Profunctor p => Optic_ m p i (Curry is i) s t u v
km = joinKinds @k @l @m @p k
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These joinKinds are essentially the same as castOptic? Should we have castOptic-like function which uses joinKinds?

Do we need Is at all, isn't Is k l JoinKinds k l l? I don't suggest we need to remove Is, I just wonder how JoinKinds is related.

Copy link
Copy Markdown
Collaborator Author

@arybczak arybczak Feb 9, 2021

Choose a reason for hiding this comment

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

These joinKinds are essentially the same as castOptic?

Yes, it's basically an extended version of implies you can use for both k and l.

Should we have castOptic-like function which uses joinKinds?

Is is used for expressing a subtyping relation, while JoinKinds for composition. It's true that they look very similar, but have distinct use cases.

Do we need Is at all, isn't Is k l JoinKinds k l l?

Yes, it looks like it. Functionally it could be used in place of Is, but it would be weird to users. Plus error messages in overlappable instances are different.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree that their use cases are different, and thus repeating is not bad (especially as it is codegen).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a reason to prefer this encoding over reusing Is like this?

class (Is k m, Is l m) => JoinKinds k l m | k l -> m

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

JoinKinds won't be newtype anymore. It might affect optimizer. If it doesn't, I'd like this sanity check.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Is there a reason to prefer this encoding over reusing Is like this?

class (Is k m, Is l m) => JoinKinds k l m | k l -> m

We'd need to enumerate all cases anyway and we use the same codegen for it as for the Is instances.

I also don't know if there can be any complications like errors from Is being printed when overlappable error instance for JoinKinds is picked.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fair points. I'm happy with the plan here, feel free to merge.

I can imagine one might know JoinKinds and yet need Is, but only if one is doing something pretty close to optics internals, so let's worry about that if we ever find such a case.

In another context (HasField-related) I find myself wishing that one could define magically-contracted constraint synonyms, e.g. define

type Is k l = JoinKinds k l l

and have GHC print Is instead of JoinKinds in inferred types and error messages whenever they are equivalent. Perhaps one day...

@arybczak
Copy link
Copy Markdown
Collaborator Author

arybczak commented Feb 9, 2021

BTW I noticed that type signatures before were quadratic in terms of length of the pipeline, since e.g. in a signature for #pets % traversed % #_GoldFish % #name of length 4 you can see 3 nested joins, 2 nested joins and 1 nested join (multiple times actually).

The typeclass makes them linear.

@arybczak arybczak mentioned this pull request Feb 11, 2021
Copy link
Copy Markdown
Member

@adamgundry adamgundry left a comment

Choose a reason for hiding this comment

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

This generally looks good to me. I guess we are losing a small amount of expressiveness by dropping the type family, e.g. you needed to (re)introduce ReadOnlyOptic, but I'm inclined to think the more explicit approach is better. The type inference improvements are very welcome!

Optic k % Optic l = Optic m
where
km :: forall p i. Profunctor p => Optic_ m p i (Curry is i) s t u v
km = joinKinds @k @l @m @p k
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a reason to prefer this encoding over reusing Is like this?

class (Is k m, Is l m) => JoinKinds k l m | k l -> m

@arybczak
Copy link
Copy Markdown
Collaborator Author

I'll merge after the weekend if there are no objections.

@arybczak arybczak merged commit a64caaf into master Feb 15, 2021
@arybczak arybczak deleted the join-kinds branch February 15, 2021 16:17
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.

3 participants