Skip to content

Eq1 & Ord1 deriving#2703

Closed
garyb wants to merge 2 commits intopurescript:masterfrom
garyb:eq-ord-1-deriving
Closed

Eq1 & Ord1 deriving#2703
garyb wants to merge 2 commits intopurescript:masterfrom
garyb:eq-ord-1-deriving

Conversation

@garyb
Copy link
Copy Markdown
Member

@garyb garyb commented Mar 4, 2017

Resolves #2702

Well, just Eq1 deriving so far... thought I'd open this for discussion before proceeding.

The implementation is a little janky perhaps, I'm not sure how else we could approach this, without having information about the instances the other types have already. Instead of examining the constraints on the instance, ideally we'd check the environment to see whether there's an Eq1 instance for the thing we're comparing, and switch based on that - obviously we can't do that right now though.

If the current approach seems acceptable enough for now I'll do the same for Ord1. Maybe also should look at doing something like eq1 = eq if both instances are being defined together, to cut down on the amount of code that is generated (will have to examine the instance constraints a bit for this I guess).

| isEq1Constrained ty = preludeEq1 l r
| otherwise = preludeEq l r

isEq1Constrained (TypeApp f _) = any (\Constraint{..} -> constraintArgs == [f]) deps
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Whoops, need to check what the constraint is for here!

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Mar 4, 2017

I was thinking the derived instances would just look like eq1 = eq. Would that be straightforward?

@garyb
Copy link
Copy Markdown
Member Author

garyb commented Mar 6, 2017

I was thinking the derived instances would just look like eq1 = eq. Would that be straightforward?

That's what I'm proposing for the cases there is an Eq instance already. We could just make that a requirement though, and that's fine too - I still think the ability to take advantage of Eq1 constraints in Eq / Eq1 deriving is useful though.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Mar 6, 2017

I still think the ability to take advantage of Eq1 constraints in Eq / Eq1 deriving is useful though.

Can you elaborate? I'm not sure what you mean.

@garyb
Copy link
Copy Markdown
Member Author

garyb commented Mar 6, 2017

So in the code I have here, if there's an Eq1 constraint on a type, it will use eq1 when comparing rather than eq. Say you have a constructor like this:

data Foo f a 
  = Foo (f Int) a
  | Bar (f String) a
  | Baz (f a)

You can just derive eq for that with (Eq1 f, Eq a) => Eq (Foo f a) if Eq1 constraints are usable.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Mar 6, 2017

Another option might be to define data App f a = App (f a) with an instance (Eq1 f, Eq a) => Eq (App f a), and then to always use Eq. If the user wants to derive using Eq1, they could use App.

That would be like the trick of using Flip to derive a functor instance for an argument which is not the last argument. Not exactly elegant, but it reduces the amount of code, compared to special-casing it in the compiler.

Another option could be to use Eq1 when we see a type variable applied to some argument.

Let me think about this a bit more though.

@garyb
Copy link
Copy Markdown
Member Author

garyb commented Mar 6, 2017

Not exactly elegant, but it reduces the amount of code, compared to special-casing it in the compiler.

I don't think I follow you there? How could introducing a bunch of extra type-juggling stuff like App be less code than the derived Eq calling eq1 when it appears to be suitable according to the provided constraints?

You don't really need Eq1 to deal with that case I gave an example of earlier, it's just you end up with an awful set of constraints if you don't: (Eq a, Eq (f String), Eq (f Int), Eq (f a)) => Eq (Foo f a), but I'd probably use that over something like App if I had no other choice, since it would be less painful to deal with everywhere else the type is used.

@garyb
Copy link
Copy Markdown
Member Author

garyb commented Mar 6, 2017

Unless we had pattern synonyms that is, that's the only thing that would make the Flip / App style tricks a realistic prospect for me 😄

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Mar 6, 2017

How could introducing a bunch of extra type-juggling stuff like App be less code than the derived Eq calling eq1

I just meant less compiler code, but it's not a big deal, I'm just thinking about other possible approaches. I think this is a fine approach. I would rather case on the syntax of the terms than the syntax of the constraint context, if possible, which is why I suggested looking for applications of a type variable to some other type.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Mar 23, 2017

@garyb What do you think about the idea of using eq1 when an argument is of type f a for some type variable f, instead of looking in the context?

Also, I think it would be cool to add an example for newtype Rec f = Rec (f (Rec f)) since we used to have trouble with infinite instances for that.

@garyb
Copy link
Copy Markdown
Member Author

garyb commented Mar 23, 2017

What do you think about the idea of using eq1 when an argument is of type f a for some type variable f, instead of looking in the context?

I think we might run into trouble with that - I don't think it's possible to make every type constructor Eq1 - Compose, for example.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Mar 23, 2017

Not really the point, I know, but Compose does have an Eq1 instance, you just need an adapter to go back to Eq (g a) and an unfortunate Functor constraint:

http://try.purescript.org/?gist=375025e006ceccd9975f89279e347a78

(I think you really don't need the Functor constraint actually, since App is a newtype, so you could just use unsafeCoerce, but still...)

@garyb
Copy link
Copy Markdown
Member Author

garyb commented Mar 23, 2017

Oh! Cool, I didn't think of that, and I tried for quite a while to come up with something too 😕

How would we make this work then? I would have thought the Eq1 constraint would be needed on an instance anyway, just as Eq is needed for something like Box a => Eq (Box a). Are we saying that the constraint is still needed, but just using the kind to determining whether eq1/ compare1 should be used instead? I mean, I'm fine with that, just not sure what it gains us over checking the constraints.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Mar 24, 2017

I would have thought the Eq1 constraint would be needed on an instance anyway

Yes, it'll be needed. I just want to explore options, and usually these things have been directed by the structure of the types, not the context, so I just wanted to go over pros and cons.

@garyb
Copy link
Copy Markdown
Member Author

garyb commented Mar 25, 2017

So, if we were doing this based on the kind of types, would it be a case of using eq1 for anything k -> Type -> Type? Say we had this nonsense:

newtype Weird a f = Weird (f a)

data MySum a
  = Value a
  | CouldBe (Maybe a)
  | HowAbout (Either a String)
  | Fuse (Coyoneda MySum a)
  | Pointless (Weird a MySum)

If we derived Eq for MySum, it'd try to use:

case comparison
Value eq
CouldBe eq1
HowAbout eq1
Fuse eq1
Pointless eq

?

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Mar 25, 2017

I'm suggesting only using eq1 when a type variable is applied to a type. The only case there would be the f a in Weird.

@garyb
Copy link
Copy Markdown
Member Author

garyb commented Mar 25, 2017

Hello, I'm Gary, and I miss extremely obvious points sometimes.

@garyb
Copy link
Copy Markdown
Member Author

garyb commented Jun 25, 2017

I've not forgotten about this one, I'll be working on it again soon.

@parsonsmatt
Copy link
Copy Markdown
Contributor

What's left to do here? 🙂 Implementing Ord1 in the same manner as Eq1, or does Eq1s implementation need to change?

@garyb
Copy link
Copy Markdown
Member Author

garyb commented Jan 20, 2018

Will revive shortly.

@garyb garyb closed this Jan 20, 2018
@garyb garyb mentioned this pull request Jan 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants