Skip to content

Remove old generics and generics-rep record support #3007

New issue

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

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

Already on GitHub? # to your account

Merged
merged 4 commits into from
Sep 14, 2017

Conversation

paf31
Copy link
Contributor

@paf31 paf31 commented Jul 30, 2017

This removes

  • the old generic deriving mechanism
  • deriving for records in generics-rep

It needs some changes to generics-rep, and we'd probably want to deprecate generics too.

This is breaking, so it would go into 1.0 or 0.12.0.

Finally, this means we need to figure out how to derive Eq, Show, Ord, etc. for records using RowToList, which means moving some things around in the core libraries.

@hdgarrood
Copy link
Contributor

I was expecting that we wouldn't derive Eq, Show, Ord etc, but rather just define them as normal instances; is that not the case? What are the unresolved questions around instances such as Show (Record a)?

@hdgarrood
Copy link
Contributor

Also, this seems fairly significant, so I think it might be sensible to call the first release containing this v0.12.0, just in case we later notice some problem with it which we want to fix, but fixing it requires another breaking change.

@paf31
Copy link
Contributor Author

paf31 commented Jul 30, 2017

Yes, sorry I meant we should write instances for those.

And yes, I think we do need a 0.12.0 release separate from 1.0.0 at this point.

@hdgarrood
Copy link
Contributor

Is there anything stopping us from writing those instances now, and releasing updated core libraries with those instances, so that people can use them with the 0.11.x compiler?

@paf31
Copy link
Contributor Author

paf31 commented Jul 30, 2017

Yes, we'd need to move RowToList somewhere where the new instances in Prelude could use it.

@paf31
Copy link
Contributor Author

paf31 commented Jul 30, 2017

Fixes #2911.

Copy link
Contributor

@hdgarrood hdgarrood left a comment

Choose a reason for hiding this comment

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

Would it make sense to add a test which ensures that code which attempts to derive an old-style Generic instance fails with the correct error? I suppose we probably have a test for that already.

@paf31
Copy link
Contributor Author

paf31 commented Sep 14, 2017

@hdgarrood Done, thanks! I'll merge once CI passes.

@paf31 paf31 merged commit fe6a098 into master Sep 14, 2017
@paf31 paf31 deleted the phil/remove-old-generics branch September 14, 2017 19:47
@joneshf
Copy link
Member

joneshf commented Sep 15, 2017

Would it be possible to add a Fail instance to generics/generics-rep describing this particular change and the resolution?

@paf31
Copy link
Contributor Author

paf31 commented Sep 15, 2017

How would it work? The compiler won't derive instances for records any more, so the issue is not that we don't want an instance to exist, but that we want other instances to exist (e.g. Show (Record r)) now.

@joneshf
Copy link
Member

joneshf commented Sep 15, 2017

I see. What sort of message do you get when you try to derive now?

@paf31
Copy link
Contributor Author

paf31 commented Sep 15, 2017

No instance for YourClass { ... }

@fsoikin
Copy link
Contributor

fsoikin commented May 14, 2018

What's the official way of migrating old Data.Generic-based code to Data.Generic.Rep? Is there, perhaps, a stopgap instance New.Generic a r => Old.Generic a implemented somewhere?

@paf31
Copy link
Contributor Author

paf31 commented May 14, 2018

You could try using this code. If it doesn't get merged, I'll probably close the PR and make a generics-compat repo.

@fsoikin
Copy link
Contributor

fsoikin commented May 14, 2018

Got it, thanks @paf31.

One question, out of curiosity though: a comment on that PR states that "For standard data structures, you can simply write derive instance deepFoo :: Deep Foo in the module they are declared, and the instance methods will be filled in for you." How does that work exactly? PureScript can't do DeriveAnyClass, can it?

@paf31
Copy link
Contributor Author

paf31 commented May 14, 2018

I may have copied that incorrectly from the original docs 😄

@fsoikin
Copy link
Contributor

fsoikin commented May 14, 2018

Ok, got it.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants