network icon indicating copy to clipboard operation
network copied to clipboard

PatternSynonyms vs Show/Read

Open kazu-yamamoto opened this issue 4 years ago • 16 comments

PatternSynonyms solves the problem of closed sum types. However, it introduced a possible backward compatibility issue relating to Show and Read:

> import Network.Socket
> ReuseAddr 
SockOpt {sockOptLevel = 65535, sockOptName = 4}
> show ReuseAddr 
"SockOpt {sockOptLevel = 65535, sockOptName = 4}"
> read "ReuseAddr" :: SocketOption 
<interactive>:4:1: error:
    • No instance for (Read SocketOption) arising from a use of ‘read’
    • In the expression: read "ReuseAddr" :: SocketOption
      In an equation for ‘it’: it = read "ReuseAddr" :: SocketOption

It's easy to fix this issue by adding Read and Show instance by hand. I would like to discuss it's worth doing.

Relating to #459.

Cc: @vdukhovni, @eborden

kazu-yamamoto avatar May 25 '20 00:05 kazu-yamamoto

One approach is:

  • Keep the perfect backward compatibility in 3.1
  • Throw away the compatibility in 3.2

That is, in 3.1:

data SocketOption = SockOpt {
    sockOptLevel :: !CInt
  , sockOptName  :: !CInt
  } deriving (Eq)

instance Show SocketOption where
  show = showSocketOption

showSocketOption ReuseAddr = "ReuseAddr"
...

And then, in 3.2:

data SocketOption = SockOpt {
    sockOptLevel :: !CInt
  , sockOptName  :: !CInt
  } deriving (Eq, Show)

I wonder how to warn show and read to SocketOption in 3.1.

kazu-yamamoto avatar May 25 '20 00:05 kazu-yamamoto

Cc: also @infinity0

kazu-yamamoto avatar May 25 '20 00:05 kazu-yamamoto

@eborden Note that we are planning to introduce PatternSynonyms to Family etc.

kazu-yamamoto avatar May 25 '20 00:05 kazu-yamamoto

The variant with showSocketOption ReuseAddr = "ReuseAddr" is certainly a lot more readable if it ever crops up in an error message, ... having put in the effort with Family and SocketType my opinion is I guess biased, but am inclined to say this is worth doing, and even the Read instance turned out to be easier than I thought it would be.

vdukhovni avatar May 25 '20 01:05 vdukhovni

Anyone else?

vdukhovni avatar May 28 '20 02:05 vdukhovni

@vdukhovni Let's add custom Read instances. I think that everything is necessary for SocketType but only important values are needed for Family.

kazu-yamamoto avatar Jun 01 '20 00:06 kazu-yamamoto

@vdukhovni Please don't use pure at this moment. Pleas use return instead.

kazu-yamamoto avatar Jun 01 '20 00:06 kazu-yamamoto

@vdukhovni Please don't use pure at this moment. Pleas use return instead.

Just curious, can you explain why?

vdukhovni avatar Jun 01 '20 01:06 vdukhovni

For backward compatibility. Probably we can throw it away now. But we decided to use return in v3.1.

kazu-yamamoto avatar Jun 01 '20 08:06 kazu-yamamoto

For backward compatibility. Probably we can throw it away now. But we decided to use return in v3.1.

I thought the upcoming release requires at least GHC 8.0, and that return a == pure a in all GHC 8.x releases. Am I not remembering right?

vdukhovni avatar Jun 01 '20 10:06 vdukhovni

I think it is reasonable to stay with return in v3.1 series and use pure if necessary in future major versions.

kazu-yamamoto avatar Jun 02 '20 00:06 kazu-yamamoto

@vdukhovni Would you like to add custom Read instances in v3.1.2?

kazu-yamamoto avatar Jul 01 '20 06:07 kazu-yamamoto

@vdukhovni Would you like to add custom Read instances in v3.1.2?

I didn't know it was up to me. If nobody else is willing to do it, I'll see what I can do...

vdukhovni avatar Jul 01 '20 07:07 vdukhovni

Sorry for the ambiguity. I guess that you are the best person to implement it.

kazu-yamamoto avatar Jul 02 '20 02:07 kazu-yamamoto

@vdukhovni recruited me to work on implementing new read/show instances for the various types still missing them, and i went ahead and developed a more robust paradigm for easily implementing many without doubling up on boilerplate pattern-matches for each instance.

I also caught a bug in Network.Socket.Options.

Both my WIP changes and the bug removal are included in a pull request I just filed. #465

今後よろしくお願いいたします。

archaephyrryx avatar Jul 05 '20 23:07 archaephyrryx

@archaephyrryx Thanks.

こちらこそ、よろしくお願いします。

kazu-yamamoto avatar Jul 06 '20 23:07 kazu-yamamoto