Opened 15 months ago

Closed 14 months ago

Last modified 14 months ago

#9069 closed feature request (fixed)

-XDeriveTraversable should imply -XDeriveFunctor and -XDeriveFoldable

Reported by: sjoerd_visscher Owned by:
Priority: normal Milestone: 7.10.1
Component: Compiler Version: 7.8.2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description

In the discussion in #2953 it was concluded that it would be best to have separate flags for deriving Functor, Foldable and Traversable. I agree with that conclusion, but it seems that it wasn't considered to have -XDeriveTraversable imply -XDeriveFunctor and -XDeriveFoldable. I can't think of a reason not to want that.

Attachments (3)

patch-9069.diff (2.1 KB) - added by sjoerd_visscher 15 months ago.
patch-9069.2.diff (3.3 KB) - added by sjoerd_visscher 15 months ago.
language-options-docs-patch.diff (69.9 KB) - added by sjoerd_visscher 14 months ago.
Updates to the documentation

Download all attachments as: .zip

Change History (18)

comment:1 Changed 15 months ago by sjoerd_visscher

I.e. add this to the impliedFlags list:

    , (Opt_DeriveTraversable, turnOn, Opt_DeriveFunctor)
    , (Opt_DeriveTraversable, turnOn, Opt_DeriveFoldable)

Changed 15 months ago by sjoerd_visscher

comment:2 Changed 15 months ago by sjoerd_visscher

  • Status changed from new to patch

comment:3 Changed 15 months ago by sjoerd_visscher

Note: this is my first GHC patch!

comment:4 Changed 15 months ago by simonpj

Ah -- but you forgot to update the user manual!

Also can you just articulate the reasoning behind having -XDeriveTraversable imply the other two, and perhaps summarise that reasoning in the user-manual entry?

Thanks

Simon

comment:5 Changed 15 months ago by sjoerd_visscher

Right, thanks, I will do that!

The main reason is that currently you have to quite a bit to just get a derived Traversable instance:

  1. You have to add deriving (Functor, Foldable, Traversable)
  2. You have to import Data.Foldable and Data.Traversable
  3. You have to turn on 3 extensions.

Item 1 is fine, we don't want any magic. Item 2 could be solved by moving Foldable and Traversable to the prelude. So the only thing that I can do right now is improve item 3.

If you derive the Traversable instance it would be invalid to have an instance for Functor or Foldable that is not equiavalent to the derived instances, so it makes sense to turn those extensions on automatically.

Changed 15 months ago by sjoerd_visscher

comment:6 Changed 15 months ago by sjoerd_visscher

That second patch replaces the first. (I forgot to check the checkmark, and there seems to be no way to fix it afterwards.)

comment:7 Changed 15 months ago by sjoerd_visscher

By the way, how can I test the documentation? Edit: Never mind, found the appropriate page on the wiki...

Last edited 15 months ago by sjoerd_visscher (previous) (diff)

comment:8 Changed 15 months ago by simonpj

OK, thanks.

You missed the flag reference summary list Section 4.20.12, where flag implications should be summarised. But that's not surprising since DeriveTraversable and friends aren't even listed there.

Could you do us a favour?

  • Add the DeriveX extensions to that table
  • Put the whole table in alphabetical order of extension name
  • Check that there are no more missing one
  • Check that all implications are listed.

That would be a very useful service. It could be a separate patch.

Meanwhile, Austin, please commit.

Simon

Changed 14 months ago by sjoerd_visscher

Updates to the documentation

comment:9 Changed 14 months ago by sjoerd_visscher

It might be a good idea to not to wait too long to merge the documentation patch in. Because the whole language options table is reordered any change to it will create a merge conflict.

comment:10 Changed 14 months ago by simonpj

Would someone at ZuriHac like to do this?

comment:11 Changed 14 months ago by nomeata

Will have a look.

comment:12 Changed 14 months ago by Sjoerd Visscher <sjoerd@…>

In 3bdc78b520c05706f8b66033b154c07ed021ac33/ghc:

Make DeriveTraversable imply DeriveFunctor/Foldable

Implements #9069

comment:13 Changed 14 months ago by Sjoerd Visscher <sjoerd@…>

In 63d7047a8a20c4e8b1c050b4577bbd7ee5cebafc/ghc:

Added testcase for #9069

comment:14 Changed 14 months ago by nomeata

  • Resolution set to fixed
  • Status changed from patch to closed

Looked fined and validated; pushed.

comment:15 Changed 14 months ago by hvr

  • Milestone set to 7.10.1
Note: See TracTickets for help on using tickets.