Skip to content

Remove old generics and generics-rep record support#3007

Merged
paf31 merged 4 commits into
masterfrom
phil/remove-old-generics
Sep 14, 2017
Merged

Remove old generics and generics-rep record support#3007
paf31 merged 4 commits into
masterfrom
phil/remove-old-generics

Conversation

@paf31

@paf31 paf31 commented Jul 30, 2017

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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

paf31 commented Jul 30, 2017

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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

paf31 commented Jul 30, 2017

Copy link
Copy Markdown
Contributor Author

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

@paf31

paf31 commented Jul 30, 2017

Copy link
Copy Markdown
Contributor Author

Fixes #2911.

@hdgarrood hdgarrood left a comment

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.

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

paf31 commented Sep 14, 2017

Copy link
Copy Markdown
Contributor Author

@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

joneshf commented Sep 15, 2017

Copy link
Copy Markdown
Member

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

@paf31

paf31 commented Sep 15, 2017

Copy link
Copy Markdown
Contributor Author

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

joneshf commented Sep 15, 2017

Copy link
Copy Markdown
Member

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

@paf31

paf31 commented Sep 15, 2017

Copy link
Copy Markdown
Contributor Author

No instance for YourClass { ... }

@fsoikin

fsoikin commented May 14, 2018

Copy link
Copy Markdown
Contributor

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

paf31 commented May 14, 2018

Copy link
Copy Markdown
Contributor Author

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

fsoikin commented May 14, 2018

Copy link
Copy Markdown
Contributor

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

paf31 commented May 14, 2018

Copy link
Copy Markdown
Contributor Author

I may have copied that incorrectly from the original docs 😄

@fsoikin

fsoikin commented May 14, 2018

Copy link
Copy Markdown
Contributor

Ok, got it.

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.

5 participants