Add more instances.

[TravisWhitaker]

Adds instances for:

• UArray
• ForeignPtr
• TVar

Fixes #44 and fixes #15. Note that the ForeignPtr instance is strict in the contained Addr# but not the ForeignPtrContents. I think this is fine, since the only pure computation that can be done on a ForeignPtr is address arithmetic.

[RyanGlScott]

Thanks! Can you also add an entry to the changelog.md about this?

[RyanGlScott]

Great, thanks @TravisWhitaker.

@hvr, does this look good to you?

[TravisWhitaker]

Pinging @hvr

[TravisWhitaker]

What is required to merge this?

[RyanGlScott]

I haven't merged this yet since I was reminded of an ongoing discussion about whether deepseq should provide NFData for "reference-like" types (such IORef and (->)) at all. @hvr (the principal maintainer of deepseq) has expressed skepticism about this, so I'm hesitant to proceed further without his input.

FWIW, the only instances in this patch that would fall into the "reference-like" category are the TVar and ForeignPtr instances. The UArray instance seems uncontroversial, so perhaps that should be in its own patch.

[TravisWhitaker]

I don't see how the instances for TVar and ForeignPtr (and IORef) are anything like an instance for (->) r. All of the former types are explicit references; there's no good reason IMO to expect that a normal form reference can only refer to normal form data.

Precisely what it means to have a normal form value of type (->) r seems murky to me. What it means to have a normal form value of type TVar, Ptr, ForeignPtr, STRef, or IORef seems pretty straight forward in contrast: there's no thunk evaluation required to determine where the reference is pointing.

[cartazio]

I think the point is that for most other structures the definitions of the provided instances pass through to the child types. If we had some sort of non recursive marker new type say NonRec we could perhaps have instances like NonRec (Ioref a)?

…

On Thu, Oct 31, 2019 at 6:43 PM Travis Whitaker ***@***.***> wrote: I don't see how the instances for TVar and ForeignPtr (and IORef) are anything like an instance for (->) r. All of the former types are explicit references; there's no good reason IMO to expect that a normal form reference can only refer to normal form data. Precisely what it means to have a normal form value of type (->) r seems murky to me. What it means to have a normal form value of type TVar, Ptr, ForeignPtr, STRef, or IORef seems pretty straight forward in contrast: there's no thunk evaluation required to determine where the reference is pointing. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#50?email_source=notifications&email_token=AAABBQT6BMJQL2IXFKWFJDTQRNNQ3A5CNFSM4IY44WGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECZPA3Q#issuecomment-548597870>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAABBQR3SAPLQEFM6ZVHWILQRNNQ3ANCNFSM4IY44WGA> .

[TravisWhitaker]

I think the point is that for most other structures the definitions of the provided instances pass through to the child types.

What's a "child type"? If it's just a type constructor's argument(s), would you consider the instances for Ptr, FunPtr, Fixed, StableName, MVar, Proxy, :~:, and :~~: problematic too?

[TravisWhitaker]

@RyanGlScott @hvr what are the next steps for this PR?

[TravisWhitaker]

Pinging @RyanGlScott @hvr how can this be moved forward?

[SamuelSchlesinger]

I think the point is that for most other structures the definitions of the provided instances pass through to the child types. If we had some sort of non recursive marker new type say NonRec we could perhaps have instances like NonRec (Ioref a)?

I like this approach. One could define NFRec and NonNFRec wrappers and the relevant instances in a completely separate package, as the maintainers of this library seem potentially uninterested in this PR moving forward at the moment.

[cartazio]

We do need to widen the contributor agency and I’m definitely guilty of it in my own maintainer duties. I’m glad you like it! (The Rec/no Rec idea) I’m not quite sure how I’d structure it as a tool within the current api mind you. But it seems like a useful vocab that’s implicit in the underlying discussion. I think it’s mostly cognitive fatigue of worrying about the Cartesian product of interactions and breakages if a bad semantics slips through. I could be wrong. I find that when cognitive fatigue is the blocker, the simplest way to help progress things it to do some mix of the following 1) help write down / systematize the concerns or information, if it can be tabulated for comparison/ consistency checking all the Better 2) for code where I’m concerned about breakage, I actually have done a Grep of allll of hackage to look at / characterize how the impacted api is used and what might break so I can plan patches or remediation / assess impact. (Easiest when there’s a distinct module import or unique operation name ) 3) establish feeling confident about the semantics being correct. This one I don’t have a good guidance on. The point being, I think it’s best to view the slow velocity here not as adversarial, but rather as a cognitive load issue where evidential decision support will help progress the discussion. Also fun collab makes everything more fun and effective!

…

On Thu, Dec 26, 2019 at 4:37 PM Samuel Schlesinger ***@***.***> wrote: I think the point is that for most other structures the definitions of the provided instances pass through to the child types. If we had some sort of non recursive marker new type say NonRec we could perhaps have instances like NonRec (Ioref a)? I like this approach. One could define NFRec and NonNFRec wrappers and the relevant instances in a completely separate package, as the maintainers of this library seem potentially uninterested in this PR moving forward at the moment. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#50?email_source=notifications&email_token=AAABBQVISWYNBLSPP76RPL3Q2UPY7A5CNFSM4IY44WGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHWFSJY#issuecomment-569137447>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAABBQXC2QDUFVECE7XSVDLQ2UPY7ANCNFSM4IY44WGA> .

[cartazio]

@andrewthad @chessai thoughts?

[chessai]

I didn't see this, sorry. I like the wrapper idea. I think the addition of those, plus expanding documentation would work well and minimise breakage. I am specifically talking about reference types, not (->).

[cartazio]

ok, so would we do eg

newtype NoRecWHNF a = NRWHNF a
instance NFData (NoRecWHNF a) where
   rnf (NRWHNF a) = rwhnf a 

and say "use this"?

[chessai]

Yep, that's what i was thinking of. The added NFData instances (different from what's in this PR) would go to NF (including the inner type), but the NoRec would always just go to WHNF. Are we on the same page?

[cartazio]

Ok. So you’re proposing that nfdata instances should only be provided for things we can fully normalize (which perhaps includes flat structures like storable or boxed vectors.) and anything else should go through a marker that says “sorry, only whnf, no recursion”?

Or could you unpack the intent and execution and present vs future state I think you’re proposing?

[TravisWhitaker]

I'm confused about what the Rec/NoRec wrappers have to do with the instances proposed in this PR. UArray, ForeignPtr, and TVar are not recursive types, and although their type constructors take arguments, whether or not values of type UArray i a/ForeignPtr a/TVar a are in normal form have nothing to do with what the argument type a is, or indeed any values of type a. These types have no a in their structures at all; for example, I can easily construct a value of type ForeignPtr Void that is in normal form.

I've been working with this definition of "normal form:" values in normal form can not be reduced any further, i.e.

• The constructor is determined.
• The constructor's arguments are not thunks.
• The constructor's arguments are in normal form.

This is subtle in the case of some reference types, since the reference's constructors don't contain the referenced type. Consider ForeignPtr for example. Values of type ForeignPtr a may reference values of type a, even though the ForeignPtr constructor and its arguments don't hold a value of type a. Because a ForeignPtr a is really just an Addr# with some finalizers, we can use the exact same normalization strategy no matter what a is. The normalized values yielded by the instances in this PR are not in WHNF even though rwhnf is a sufficient implementation (except for ForeignPtr, which is strict in the Addr# but not the finalizers). A ForeignPtr with a known Addr# is a real normal form value, unlike, for example, the values yielded by the NFData (a -> b) instance (although I might be wrong about this, since I don't know what a function's normal form is) . Since this is a strategy that yields correct normal forms, I don't think it warrants a special wrapper.

It would be a mistake to dereference the a that a ForeignPtr might point to (e.g. nullPtr can be made into a ForeignPtr), since the referenced value has nothing to do with whether or not a ForeignPtr value is in normal form. Indeed, I have a library that would segfault if the NFData (ForeignPtr a) instance attempted to dereference and normalize the referenced a values.

I suspect that Herbert simply hasn't had the time to review this PR or participate in this conversation. I'll shoot him an email, since that seems to be his preference these days.

[TravisWhitaker]

The added NFData instances (different from what's in this PR) would go to NF (including the inner type)

@chessai, wouldn't the "recursive" NFData a => NFData (IORef a) instance require unsafePerformIO or similar? What rnf definition would you use for this instance?

[cartazio]

I agree, unsafe perform io instances are probably a bad idea

…

On Sun, Dec 29, 2019 at 10:12 PM Travis Whitaker ***@***.***> wrote: The added NFData instances (different from what's in this PR) would go to NF (including the inner type) @chessai <https://github.com/chessai>, wouldn't the "recursive" NFData a => NFData (IORef a) instance require unsafePerformIO or similar? What rnf definition would you use for this instance? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#50?email_source=notifications&email_token=AAABBQRCM633MZXRJIKKORTQ3FRJDA5CNFSM4IY44WGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHZPIAQ#issuecomment-569570306>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAABBQXSJHP5UEOHIXYO5SLQ3FRJDANCNFSM4IY44WGA> .

[chessai]

@TravisWhitaker yeah, my mistake. Didn't think too deeply on what I said about the added instances. And for most reference types, what i suggested doesn't really make any sense, as you pointed out. Perhaps the PR is just best as it stands, with sufficient documentation. To my understanding, we aren't really going to move forward without Herbert though.

Perhaps your well-worded note about reference types should be in the documentation, if that is how we will treat them?

values in normal form can not be reduced any further, i.e.
• The constructor is determined.
• The constructor's arguments are not thunks.
• The constructor's arguments are in normal form.

This is subtle in the case of some reference types, since the reference's constructors don't contain the referenced type. Consider ForeignPtr for example. Values of type ForeignPtr a may reference values of type a , even though the ForeignPtr constructor and its arguments don't hold a value of type a . Because a ForeignPtr a is really just an Addr# with some finalizers, we can use the exact same normalization strategy no matter what a is. The normalized values yielded by the instances in this PR are not in WHNF even though rwhnf is a sufficient implementation (except for ForeignPtr, which is strict in the Addr# but not the finalizers). A ForeignPtr with a known Addr# is a real normal form value, unlike, for example, the values yielded by the NFData (a -> b) instance.

EDIT: the note needs an edit, of course, if it were to be introduced into the actual documentation

[TravisWhitaker]

Pinging @hvr .

[cartazio]

Hvr: would you Like some other clc folks to take point on this ?

…

On Thu, Feb 27, 2020 at 5:07 PM Travis Whitaker ***@***.***> wrote: Pinging @hvr <https://github.com/hvr> . — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#50?email_source=notifications&email_token=AAABBQUB35P5GSNA4ALWIHLRFA2SVA5CNFSM4IY44WGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENGENTA#issuecomment-592201420>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAABBQRRJJSTNIXRJ7SCQITRFA2SVANCNFSM4IY44WGA> .

[TravisWhitaker]

Pinging @hvr.

@RyanGlScott is there anything that can be done to move this forward? I'm really curious to hear Herbert's take on this, but it's rather unfortunate for something like this to be bottlenecked for months on one person.

[RyanGlScott]

I'm no longer on the CLC, so I can't really say one way or the other.

[TravisWhitaker]

@sjakobi @chessai what more can be done to either:

• Move this forward?
• Clearly explain and document why this should not be done?

For what it's worth, I expanded on my line of reasoning for the existence of these instances here.

[sjakobi]

@TravisWhitaker I'm not a maintainer of this package.

[chessai]

HVR is unavailable. Pinging @bgamari and @ekmett and @emilypi and @gwils for thoughts.

[mixphix]

@TravisWhitaker I've just dropped support for GHC 7 to simplify the CPP in Control.DeepSeq. Would you please rebase so that these instances can finally be added?

[TravisWhitaker]

@mixphix in light of c2432c2, I removed the UArray instance from this PR, since the design intent seems to be for array to depend on deepseq instead.

[mixphix]

Thanks for that, that is indeed the intent. Also since deepseq no longer supports versions of GHC with base <= 4.8 (corresponding to GHC < 8), the CPP you've included is unnecessary, and the import can simply be import Foreign.ForeignPtr.Safe.