Precedence-compliant Read/Show instances using Bijective pairs for patterns

[archaephyrryx]

Adapts bijective read/show boilerplate in Network.Socket.ReadShow to be precedence-sensitive while maintaining full generality of base implementation.

Re-implements bijective read/show instances for SocketOption, SocketType, Family, and CmsgId.

[vdukhovni]

Looks pretty good overall.
I'm requesting a couple of changes, and would like to see this squashed to a single commit.
Also perhaps some tests added to test round-trip correctness read (show x) == x and a few select cases of explicit show x == "expected result".

[vdukhovni]

Looks great, thanks! Basically done.

I'm asking @kazu-yamamoto to also review, and it'd be good if someone would suggest better names for _show and _parse which show and read a pair of integers, presumably wrapped by some constructor or pattern synonym. The names don't make that clear. Otherwise, this is good.

@kazu-yamamoto: The reason for the name prefix (constructor or pattern for the generic case) being optional, is that if we decide to assign patterns for, e.g., protocol numbers, then the generic case is just a literal number, because protocol numbers would have a Num instance even if they become newtype ProtocolNumber = ProtocolNumber CInt. We might then display some common protocols via their pattern names, such as TCP for 6 and UDP as 17, but most protocols would continue to display as just a number.

[kazu-yamamoto]

This looks excellent to me.

it'd be good if someone would suggest better names for _show and _parse

show' and parse'?

@archaephyrryx Could you add tests as @vdukhovni suggested?

[vdukhovni]

it'd be good if someone would suggest better names for _show and _parse

show' and parse'?

To me, these don't quite say "show/parse 2 consecutive numeric literals". So perhaps showIntX2 and parseIntX2? Though a sufficiently concise name that's clear is a bit tricky.

[archaephyrryx]

it'd be good if someone would suggest better names for _show and _parse

show' and parse'?

To me, these don't quite say "show/parse 2 consecutive numeric literals". So perhaps showIntX2 and parseIntX2? Though a sufficiently concise name that's clear is a bit tricky.

One difficulty is that the two functions convert between tuples of Integer-like values and strings containing a space-separated sequence of possibly-parenthesized integers. The tuple is necessary for _parse to be able to return two values so it is not natural to curry the input for _show, but any reference to 'tuple' in the name would be misleading as the string output of _show and the expected input for _parse are not represented as tuples.

[archaephyrryx]

I also plan on squashing several of the commits before this PR gets merged, once all outstanding concerns have been settled. Unless I am mistaken, rewriting the history of this branch would wipe the conversation history of this PR, so I am planning to hold off until things are resolved. In any case, instead of merging right away, let me know that you intend to merge so that I can reorganize the commits more naturally.

[vdukhovni]

To me, these don't quite say "show/parse 2 consecutive numeric literals". So perhaps showIntX2 and parseIntX2? Though a sufficiently concise name that's clear is a bit tricky.

One difficulty is that the two functions convert between tuples of Integer-like values and strings containing a space-separated sequence of possibly-parenthesized integers. The tuple is necessary for _parse to be able to return two values so it is not natural to curry the input for _show, but any reference to 'tuple' in the name would be misleading as the string output of _show and the expected input for _parse are not represented as tuples.

Yes, getting the name to precisely capture the input/output types is difficult, and largely impractical. I still think that showIntX2 and parseIntX2 is about the best we can do, it captures the leaf data type and multiplicity, and leaves the "shape" (tuple) deliberately vague, because we're converting between a tuple (i, j) and a curried constructor application "Cstr istr jstr".

[archaephyrryx]

To me, these don't quite say "show/parse 2 consecutive numeric literals". So perhaps showIntX2 and parseIntX2? Though a sufficiently concise name that's clear is a bit tricky.

One difficulty is that the two functions convert between tuples of Integer-like values and strings containing a space-separated sequence of possibly-parenthesized integers. The tuple is necessary for _parse to be able to return two values so it is not natural to curry the input for _show, but any reference to 'tuple' in the name would be misleading as the string output of _show and the expected input for _parse are not represented as tuples.

Yes, getting the name to precisely capture the input/output types is difficult, and largely impractical. I still think that showIntX2 and parseIntX2 is about the best we can do, it captures the leaf data type and multiplicity, and leaves the "shape" (tuple) deliberately vague, because we're converting between a tuple (i, j) and a curried constructor application "Cstr istr jstr".

Would showIntInt and parseIntInt be any better or does the 'X2' paradigm make more sense? IntInt is deliberately ambiguous between concatenation and tuple type signature, so it seems at least natural, even if not actually better than the X2 variation.

[vdukhovni]

Yes, getting the name to precisely capture the input/output types is difficult, and largely impractical. I still think that showIntX2 and parseIntX2 is about the best we can do, it captures the leaf data type and multiplicity, and leaves the "shape" (tuple) deliberately vague, because we're converting between a tuple (i, j) and a curried constructor application "Cstr istr jstr".

Would showIntInt and parseIntInt be any better or does the 'X2' paradigm make more sense? IntInt is deliberately ambiguous between concatenation and tuple type signature, so it seems at least natural, even if not actually better than the X2 variation.

I have no preference, either is fine.

[kazu-yamamoto]

I also plan on squashing several of the commits before this PR gets merged, once all outstanding concerns have been settled. Unless I am mistaken, rewriting the history of this branch would wipe the conversation history of this PR, so I am planning to hold off until things are resolved. In any case, instead of merging right away, let me know that you intend to merge so that I can reorganize the commits more naturally.

Please feel free to edit your commits (maybe by git rebase -i master) and do git push -f. I don't care even if some conversations are lost.

[archaephyrryx]

I did the rebase and the commits organized a bit better. I am probably finished with working on this PR for now, unless there are any specific adjustments you think would be appropriate. Feel free to merge whenever.

[kazu-yamamoto]

LGTM

[kazu-yamamoto]

Merged. Thank you for your excellent work!

If you have further plans, please let me know.