#9164 closed task (fixed)

Hunt down uses of `castPtr`

Reported by: nomeata Owned by:
Priority: normal Milestone:
Component: libraries/base Version: 7.8.2
Keywords: Cc: hvr, ekmett
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description

This bug is a fork from #9163:

nomeata: Slightly off topic, but maybe GHC.IO.Handle.Text can be written so that castPtr is not needed. It seems it always works with Ptr Word8, but exposes an API that uses Ptr a as a generic pointer. The type a is never needed inside GHC.IO.Handle.Text...

I tried it, and hPutBuf & Co are called with Ptr Word8 (sometimes called LitString) in all cases but BufWrite.bPutCStringLen, where we have a Ptr CChar, where CChar is a newtype. So by using coerce there (and unsafeCoerce during bootstrapping), we get the best of boths worlds: The more restrictive role and the free behaviour.

SPJ: Well regardless of the decision about the role of Ptr, what you describe sounds like an improvement anyway, getting rid of (always suspicious) castPtr calls. If you grep in the base library you'll find quite a few others. Can you eliminate them in the same way?

I think this would be a nice improvement, if you cared to push it through.

simonmar: So I've been assuming that castPtr was free all this time, and it turns out it isn't! Please make it free.

We can't change the type of hPutBuf and friends without potentially breaking code. There's no better choice than Ptr a here, because we don't know the type of the underlying data. Making it Ptr Word8 would just force the caller to use castPtr sometimes without any benefit. e.g. Ptr CChar is also a sensible choice, but it could in reality be anything.

It does look like the two castPtrs in bufWrite are unnecessary, though.

Change History (4)

comment:1 Changed 14 months ago by nomeata

We can't change the type of hPutBuf and friends without potentially breaking code.

Is there a stability guarantee on things in the GHC namespace? The module name tells me „this is lowevel stuff and may change“, at least between GHC releases and the usual base bump.

Also, given that the docs state

hPutBuf hdl buf count writes count 8-bit bytes from the buffer buf to the handle hdl. It returns ().

I’d say that Ptr Word8 is fine – that is how hPutBuf uses it, and if I want to pass a Ptr Double, I have to think about whether that works. Having to call castPtr makes me think about it.

(If you agree with this, then this might be a nice ZuriHac beginner task.)

comment:2 Changed 14 months ago by simonmar

hPutBuf and friends are re-exported by System.IO.

I’d say that Ptr Word8 is fine – that is how hPutBuf uses it, and if I want to pass a Ptr Double, I have to think about whether that works. Having to call castPtr makes me think about it.

The data in the buffer returned could be any type. Now, whether we want to represent "any type" as Ptr a or Ptr Word8 is debatable, but since we've had this API for a long time already, and changing it to Ptr Word8 could break some code, I don't see a good argument for changing it. castPtr should be free.

comment:3 Changed 14 months ago by nomeata

Ok, forgot about the re-export.

So if #9163 can be solved with castPtr being free, then this ticket becomes moot.

comment:4 Changed 14 months ago by nomeata

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

Since #9163 could be solved with castPtr being free, then this ticket becomes moot.

Note: See TracTickets for help on using tickets.