fix #7401 -- Make instance deriving for empty types same as standalone instance deriving for empty types (with EmptyDataDecls)
Needs RevisionPublic

Authored by osa1 on Jun 11 2015, 1:20 PM.

Details

Reviewers
simonpj
hvr
austin
Trac Issues
#7401
Summary

This implements the changes discussed in https://ghc.haskell.org/trac/ghc/ticket/7401. To summarize:

When EmptyDataDecls is enabled, GHC now derives same instances it would drive for empty data declarations using standalone deriving.

If EmptyDataDecls is disabled, it fails as it does without this patch.

As also asked by Simon in the trac ticket, I updated the error message. (see .stderr files)

There are some discussion about standard compliance in the trac ticket. Most notably, the standard is saying this:

If the data declaration has no constructors (i.e. when n = 0), then no classes are derivable (i.e. m = 0)

However, Simon says in the trac:

And yes, clause (6) in the ​deriving appendix explicitly dis-allows deriving( Eq ), but here I think it would be OK for GHC to be a little more generous than the standard. That is, with EmptyDataDecls (or Haskell2010) we'd accept data T deriving( Eq ) whereas H2010 would reject it. But failing to reject is very unlikely to break anyone's code.

This change requires EmptyDataDecls extension, so no diversion from the standard is happening. (I think)

One thing that I don't understand about the standard is this: Types with no constructors are syntactically allowed: https://www.haskell.org/onlinereport/haskell2010/haskellch4.html But they're still rejected by GHC, without extensions. I didn't change anything about that in this patch.


fix Trac #7401 - 3

fix Trac #7401 - adding tests

fix a name in comment

fix Trac #7401 - remove duplication

fix Trac #7401 - fix stderr of an existing file for new error message

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Branch
fix-7401
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 4401
Build 4436: GHC Patch Validation (amd64/Linux)
osa1 updated this revision to Diff 3200.Jun 11 2015, 1:20 PM
osa1 retitled this revision from to fix #7401.
osa1 updated this object.
osa1 edited the test plan for this revision. (Show Details)
osa1 added reviewers: austin, simonpj.
osa1 updated the Trac tickets for this revision.
thomie requested changes to this revision.Jun 11 2015, 2:19 PM
thomie added a reviewer: thomie.

T7401_2 never gets called, and the description doesn't explain what this patch is supposed to do. You can update the description (only) by clicking 'Edit revision' in the top right corner.

This revision now requires changes to proceed.Jun 11 2015, 2:19 PM
osa1 updated this revision to Diff 3203.Jun 11 2015, 3:06 PM
osa1 edited edge metadata.
  • run T7401_2
osa1 updated this object.Jun 11 2015, 3:18 PM
osa1 edited edge metadata.
osa1 added a comment.Jun 11 2015, 3:20 PM

@thomie, fixed T7401_2 and added some descriptions. (see also the trac ticket)

osa1 updated this object.Jun 11 2015, 3:43 PM
osa1 updated this revision to Diff 3205.Jun 11 2015, 4:55 PM
  • fix test entries in should_run/all.T
simonpj requested changes to this revision.Jun 11 2015, 5:08 PM
simonpj edited edge metadata.

Looks good to me. Except that I wonder if we should have section in the user manual about this. There was some discussion on the ticket about what the right behavior for deriving( Eq ) or deriving( Functor ) etc should be for empty data types. I'd like the user manual to document it. A new sub-section to 7.5 perhaps.

The manual can say that, with EmptyDataDecls, deriving works for all derivable classes (true)? Ie Eq, Ord, Ix, ... etc.. .Foldable, Traversable,, Functor. And it can say what code is derived, namely (I think) in every case: you get a runtime error "<method> on empty data type Foo". Is that right? If so documenting it would be good.

thanks

Simon

This revision now requires changes to proceed.Jun 11 2015, 5:08 PM
osa1 added a comment.Jun 11 2015, 6:05 PM

I'll update the user manual for this.

This patch makes deriving (...) for empty types work same as how standalone
deriving for empty types works. There are cases where standalone deriving is
failing. Examples:

With StandaloneDeriving, Ix is failing with this:

Decl.hs:10:1: error:
    Can't make a derived instance of ‘Ix Z’:
      ‘Z’ must be an enumeration type
      (an enumeration consists of one or more nullary, non-GADT constructors)
        or
      ‘Z’ must have precisely one constructor
    In the stand-alone deriving instance for ‘Ix Z’

Now it's failing with same message(except the location) with deriving (...):

Decl.hs:6:24: error:
    Can't make a derived instance of ‘Ix Z’:
      ‘Z’ must be an enumeration type
      (an enumeration consists of one or more nullary, non-GADT constructors)
        or
      ‘Z’ must have precisely one constructor
    In the data declaration for ‘Z’

Similarly for Bounded and Enum.

Foldable, Traversable, Eq, Show, Ord and many others work fine after
this patch.

Of course it's possible to make deriving Bounded, Enum etc. for empty types
working, but I think in that case we should also update how standalone deriving
works.

@simonpj, what do you think?

A note about implementation: The implementation doesn't actually make handling
of deriving (...) on empty types same with how standalone deriving is handled
for empty types. But since we can't have higher-ranked(or existential) empty
types(can we do that without constructors?), in practice they're handled the
same. (so this patch needs to be changed if we can have an empty but higher
ranked type somehow)

I can still refactor sideConditions to make deriving instances for empty
types exactly the same as how standalone deriving for empty types are done.

(Also, I can't reproduce failing tests on my system, when I try them in a fresh
directory they produce expected outputs)

thomie added a reviewer: hvr.Jun 12 2015, 2:48 AM
thomie added a subscriber: hvr.

@osa1: You can try running ./validate locally (https://ghc.haskell.org/trac/ghc/wiki/TestingPatches#Locally), and the 2 errors will most likely show up. Note that ./validate runs make maintainer-clean first, so you might want to create a second build tree for it, as explained here: https://ghc.haskell.org/trac/ghc/wiki/WorkingConventions/Git#Workingwithaseparatebuildtree.

CCing @hvr, as the Haskell2010 expert.

osa1 updated this revision to Diff 3224.Jun 12 2015, 4:23 PM
osa1 edited edge metadata.
  • trying a fix for duplicated suggestions problem
  • fix minor typo
osa1 updated this revision to Diff 3238.Jun 14 2015, 1:10 PM
osa1 edited edge metadata.
  • update user's manual
osa1 added a comment.Jun 14 2015, 1:12 PM

I fixed the validation errors and updated the user's manual.

@simonpj, also see my last comment about current behavior of non-standalone deriving.

osa1 updated this revision to Diff 3239.Jun 14 2015, 1:14 PM
  • typo
This revision now requires changes to proceed.Jun 15 2015, 3:51 AM

Could you also at some point change the revision title to something more descriptive than "fix Trac #7401"?

osa1 retitled this revision from fix #7401 to fix Trac #7401.Jun 19 2015, 8:31 PM
osa1 edited edge metadata.
osa1 retitled this revision from fix Trac #7401 to fix #7401.
osa1 retitled this revision from fix #7401 to fix #7401 -- Make instance deriving for empty types same as standalone instance deriving for empty types (with EmptyDataDecls).Jun 19 2015, 8:33 PM
thomie resigned from this revision.Aug 29 2015, 10:16 AM
thomie removed a reviewer: thomie.

Heads-up: this will probably need to be updated to account for D1168 (DeriveLift). It shouldn't be too bad, though; you'd just need to percolate your changes into gen_Lift_binds.

austin resigned from this revision.Nov 6 2017, 10:09 PM