A bit more Show-boilerplate-removal.

I won't screen this one immediately as the use of
'deriving stock' was a little bit controversial.

I have also deliberately not done it to V3.Core even
though I could, because it doesn't work with PrimContainer.
If we don't apply that, I'll do it to V3.Core too.

1 patch for repository darcs-unstable@darcs.net:screened:

patch 26b82e81504001f04a888d5c0b3d36fddb087cb4
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Mon Sep  2 14:41:19 BST 2019
  * use StandaloneDeriving for some Show instances
  
  This is possible in cases where all the directly included
  patches are concrete types rather than type variables.
  
  For example it works for
  
   data Foo p wX wY where
    Foo :: Named p wX wY -> Foo p wX wY
  
  but not for
  
   data Foo p wX wY where
    Foo :: p wX wY -> Foo p wX wY
  
  I've used DerivingStrategies and the 'stock' keyword to
  make it easier to tell at a glance what's going on given
  the increasing complexity of 'deriving'.

Attachments

• patch-preview.txt
• unnamed
• use-standalonederiving-for-some-show-instances.dpatch

> I won't screen this one immediately as the use of
> 'deriving stock' was a little bit controversial.
> 
> I have also deliberately not done it to V3.Core even
> though I could, because it doesn't work with PrimContainer.
> If we don't apply that, I'll do it to V3.Core too.
> 
> 1 patch for repository darcs-unstable@darcs.net:screened:
> 
> patch 26b82e81504001f04a888d5c0b3d36fddb087cb4
> Author: Ganesh Sittampalam <ganesh@earth.li>
> Date:   Mon Sep  2 14:41:19 BST 2019
>   * use StandaloneDeriving for some Show instances
>   
>   This is possible in cases where all the directly included
>   patches are concrete types rather than type variables.
>   
>   For example it works for
>   
>    data Foo p wX wY where
>     Foo :: Named p wX wY -> Foo p wX wY
>   
>   but not for
>   
>    data Foo p wX wY where
>     Foo :: p wX wY -> Foo p wX wY
>   
>   I've used DerivingStrategies and the 'stock' keyword to
>   make it easier to tell at a glance what's going on given
>   the increasing complexity of 'deriving'.

I am perfectly fine with GeneralizedNewtypeDeriving and also
StandaloneDeriving, I have used them myself.

As I said before, I'm not so sure about explicitly specifying a
strategy. IMO this makes sense only if we want expressly support doing
something that differs from the default strategy, and I am pretty sure
we do not want to open up that bottle. If we start mentioning strategies
now, everyone who makes a modification will be forced to observe the
existing convention and explicitly specify strategies, too. This is
quite dangerous: accidentally using the wrong strategy will lead to bugs
that are difficult to find, as there is no explicit code enywhere that
does something wrong (think of deriving Eq or Ord); and since it is just
a single word, even patch review may overlook such an error. I'd say
stick to the default and disallow explicit strategies by convention.

BTW, I am not happy about the proliferation of Haskell extensions in
single modules. I think if a developer wants to use a new extension,
there should be discussion about that and if we agree that this is a
good idea, then the extension should be enabled globally. There are a
few exceptions to this rule, e.g. UndecidableInstances should be used
only per module. For these, we should demand that enabling such an
extension requires a comment that explains why it is necessary.

GHC has acquired a lot of minor syntactical extensions over the years
that are pretty useful and utterly harmless (LambdaCase, MultiWayIf,
TupleSections to mention some) which aren't enabled by default in darcs
and we should really change that. Most of them will probably be part of
the next Haskell standard anyway.

> As I said before, I'm not so sure about explicitly specifying a
> strategy. IMO this makes sense only if we want expressly support doing
> something that differs from the default strategy, and I am pretty sure
> we do not want to open up that bottle. If we start mentioning strategies
> now, everyone who makes a modification will be forced to observe the
> existing convention and explicitly specify strategies, too. This is
> quite dangerous: accidentally using the wrong strategy will lead to bugs
> that are difficult to find, as there is no explicit code enywhere that
> does something wrong (think of deriving Eq or Ord); and since it is just
> a single word, even patch review may overlook such an error.

I have exactly the opposite view in that I think it's much easier to
overlook an implicit wrong meaning than an explicit wrong meaning. If
I'm reviewing something and I just see "deriving Foo" then I might or
might not think about what that really means. If I see "deriving stock
Foo" or "deriving anyclass Foo" or whatever then I would be much more
likely to think about what that instance would actually do.

I also see 'deriving' as just shorthand for writing things by hand. I
can at least roughly visualise what an instance would look like, as long
as I know what strategy would be used.

Anyway, it's not a big deal for this patch, I'll just send a version
without the 'stock' and 'DerivingStrategies'.

> BTW, I am not happy about the proliferation of Haskell extensions in
> single modules. I think if a developer wants to use a new extension,
> there should be discussion about that and if we agree that this is a
> good idea, then the extension should be enabled globally.

OK, that's the approach I've taken in this patch anyway. I assume from
your comments that you're ok with enabling StandaloneDeriving globally.

> There are a
> few exceptions to this rule, e.g. UndecidableInstances should be used
> only per module. For these, we should demand that enabling such an
> extension requires a comment that explains why it is necessary.

Yeah. FlexibleContexts and FlexibleInstances should perhaps also fall
into this category (they're currently globally on). There are generally
ok to use if it makes sense, but it's useful to have to stop and think
if there's a more H98-ish way first.

> GHC has acquired a lot of minor syntactical extensions over the years
> that are pretty useful and utterly harmless (LambdaCase, MultiWayIf,
> TupleSections to mention some) which aren't enabled by default in darcs
> and we should really change that.

I'm fine with it if you just want to turn those all on. We do appear to
already have LambdaCase in the darcs library.

> the next Haskell standard

Hahahahahaha

>> BTW, I am not happy about the proliferation of Haskell extensions in
>> single modules. I think if a developer wants to use a new extension,
>> there should be discussion about that and if we agree that this is a
>> good idea, then the extension should be enabled globally.
> 
> OK, that's the approach I've taken in this patch anyway. I assume from
> your comments that you're ok with enabling StandaloneDeriving globally.

Yes, definitely.

>> There are a
>> few exceptions to this rule, e.g. UndecidableInstances should be used
>> only per module. For these, we should demand that enabling such an
>> extension requires a comment that explains why it is necessary.
> 
> Yeah. FlexibleContexts and FlexibleInstances should perhaps also fall
> into this category (they're currently globally on). There are generally
> ok to use if it makes sense, but it's useful to have to stop and think
> if there's a more H98-ish way first.

I don't want to open another endless discussion about this, but I
disagree on that point. FlexibleContexts and FlexibleInstances are
absolutely essential to write modular code. For instance, we could not
define the instance FromPrim RepoPatchV3 in Darcs.Patch.V3 i.e. outside
of Darcs.Patch.V3.Core otherwise. I would be very much disappointed if
these extensions were not part of the next Haskell standard. Don't laugh!

>> GHC has acquired a lot of minor syntactical extensions over the years
>> that are pretty useful and utterly harmless (LambdaCase, MultiWayIf,
>> TupleSections to mention some) which aren't enabled by default in darcs
>> and we should really change that.
> 
> I'm fine with it if you just want to turn those all on. We do appear to
> already have LambdaCase in the darcs library.
> 
>> the next Haskell standard
> 
> Hahahahahaha

I know. I don't find it funny, though. I really wish the standardisation
process would make progress. Someone of the haskell-prime list said
everyone on the haskell prime commitee should all give up their posts,
retreat in shame and make room for others. I think he has it right. Just
my 2 cent.

> I don't want to open another endless discussion about this, but I
> disagree on that point. FlexibleContexts and FlexibleInstances are
> absolutely essential to write modular code. For instance, we could not
> define the instance FromPrim RepoPatchV3 in Darcs.Patch.V3 i.e. outside
> of Darcs.Patch.V3.Core otherwise. I would be very much disappointed if
> these extensions were not part of the next Haskell standard. Don't laugh!

I actually had that exact example in mind. The best solution would be a
FromPrim class that can be implemented for everything. Anyway, I'm fine
with the status quo.

> I know. I don't find it funny, though. I really wish the standardisation
> process would make progress. Someone of the haskell-prime list said
> everyone on the haskell prime commitee should all give up their posts,
> retreat in shame and make room for others. I think he has it right. Just
> my 2 cent.

Well, I did that several years ago and it didn't help :-)

updated version without DerivingStrategies, will screen this one

1 patch for repository darcs-unstable@darcs.net:screened:

patch 52c6cb79d887ae79a5bfc9046022ca8512cffa30
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Mon Sep  2 23:19:00 BST 2019
  * use StandaloneDeriving for some Show instances
  
  This is possible in cases where all the directly included
  patches are concrete types rather than type variables.
  
  For example it works for
  
   data Foo p wX wY where
    Foo :: Named p wX wY -> Foo p wX wY
  
  but not for
  
   data Foo p wX wY where
    Foo :: p wX wY -> Foo p wX wY

Attachments

• patch-preview.txt
• unnamed
• use-standalonederiving-for-some-show-instances.dpatch

Very nice.