[GHC] #9163: Ptr should have a phantom role

#9163: Ptr should have a phantom role ------------------------------------+------------------------------------- Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Keywords: | Operating System: Unknown/Multiple Architecture: Unknown/Multiple | Type of failure: None/Unknown Difficulty: Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | ------------------------------------+------------------------------------- In `GHC.Ptr` we see {{{ type role Ptr representational data Ptr a = Ptr Addr# deriving (Eq, Ord) }}} with no comments. Why is `Ptr` representational? In the same module we have `castPtr`: {{{ castPtr :: Ptr a -> Ptr b castPtr (Ptr addr) = Ptr addr }}} which unpacks and repacks a `Ptr`. If `Ptr` was phantom, we could use `coerce`. And that in turn would actually make a lot of code more efficient – there are lots of calls to `castPtr`. Specifically, in `nofib`, I tried implementing `castPtr` with `unsafeCoerce`. Then I found: * 12% less allocation in `reverse-complem` * 7.3% less allocation in `fasta`. * Binary sizes fell 0.1%. Both these benchmarks are ones that do a ''lot'' of I/O, and it turns out that `GHC.IO.Handle.Text.$wa1` has a `castPtr` that (all by itself) accounts for 12% of `reverse-complem`'s total allocation! So making `castPtr` free is good. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by simonpj): Richard says: It seemed clear to me that `Ptr` ''should'' be representational, thinking that we don't want to coerce a `Ptr Int` to a `Ptr Bool`. I don't see any reason why this couldn't be changed. Why is `Ptr` a `data` not a `newtype`? If it were a newtype, we could keep the role annotation and use `coerce` internally, according to the updated Coercible solver. Answer (from Simon): because the payload is an unboxed `Addr#`, so `Ptr` boxes it. A `newtype` can't have an unboxed payload. However, it is crucial that `FunPtr` have a representational argument, as normaliseFfiType' depends on this fact. There is a brief comment in `TcForeign`, but clearly more comments are necessary. Will do shortly. If we want to change `FunPtr`'s role, `normaliseFfiType'` would have to be updated, too, but it shouldn't be hard. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by simonpj): Austin says: I think Ptr should almost certainly be representational, as it is a case where the actual underlying value is the same, but what it points to is not (I'll ignore castPtr here for a second). This makes me think of something I talked about before with Andres - in general, phantom roles seem somewhat dangerous, and I kind of wonder if they should be inferred by default. Often you see some code along the lines of: {{{ module A ( Bar, newBarInt ) where data Bar a = Bar Int newBarInt :: Int -> Bar Int }}} where A is exported to the client and the module boundary enforces some restrictions on 'Bar', like what types we can instantiate `Bar a` to (the example is dumb but bear with me). In the above example, I believe the `a` in `Bar a` would be inferred at phantom role. The question I have is: what legitimate case would there be for phantom roles like this? Such usage of phantom type parameters seems very common, but in almost *all* cases I can't think of a reason why I would ever want a user to be allowed to `coerce` away the type information, if the `a` parameter was phantom. It seems like in the above example, I would almost certainly want `a` to be representational instead. What other cases exist for legitimate phantom roles such as this where we want this inference? I wonder if instead we should require an explicit role annotation for phantoms in this case - not the other way around. `Ptr` is a similar story - by default it would maybe seem reasonable to be phantom, but that seems like an especially grievous error, given we don't want people to `poke` incorrect values in or something (again, ignoring castPtr for a second, but I think the general idea holds.) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by simonpj): Well, remember that even if Ptr has a phantom role, the two types (Ptr Int) and (Ptr Double) would be different types. It's just that you could *coerce* between them. Which is perhaps not unreasonable. Indeed that is precisely what castPtr does. It’s not silent or implicit. I still don't see why it would be bad to make `Ptr` phantom. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by 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`... -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by goldfire): While I hear Austin's concerns, I tend to agree with Simon here. We don't want to consider `Ptr Int` and `Ptr Bool` to be the same types, but we ''do'' want to allow a knowledgeable programmer to get from one to the other for free. This seems to be a great use case for `coerce`. This idea extends to other cases where GHC infers phantom roles. In case it got lost, I'll ask again: why is `Ptr` a datatype, not a newtype? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by simonpj): Replying to [comment:5 goldfire]
In case it got lost, I'll ask again: why is `Ptr` a datatype, not a newtype?
See my reply in comment 1 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Changes (by simonpj): * cc: simonmar (added) Comment: Adding Simon Marlow who may want to comment on comment 4. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by nomeata): 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. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by simonpj): 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. Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by rwbarton): +1 from me on making `Ptr`'s role phantom. But especially I don't think it should be ''exactly'' representational; either phantom or nominal must be better. A `Ptr a` is not actually a pointer to a Haskell heap object of type `a`, for any type `a`. Neither the representation of the `Ptr a` itself nor the representation of the thing-being-pointed-to depends on the ''representation'' of `a` specifically. I might define a `newtype BigEndianInt = BigEndianInt Int` and write a `Storable` instance that uses a big-endian format when `peek`ing or `poke`ing. Then it isn't any more valid to cast from `Ptr Int` to `Ptr BigEndianInt` than it is to cast from `Ptr Int` to `Ptr Double`. Similar comments probably apply to Austin's hypothetical example of an inferred phantom role. If the library author does not want `Bar` to have a phantom role, they probably do not want it to have a representational role either. But it's hard to say without a more fledged-out example. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by goldfire): Helpful comments, rwbarton. Is the same true for `FunPtr`? I would assume so. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by rwbarton): Replying to [comment:11 goldfire]:
Helpful comments, rwbarton. Is the same true for `FunPtr`? I would assume so. My knee-jerk reaction is "not sure"; I am thinking that while the normal way to consume a `Ptr` is to `peek` and `poke` it, the normal way to consume a `FunPtr` is with a `foreign import ccall "dynamic"` wrapper, and I'm not sure they are obviously analogous situations.
-- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by 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 `castPtr`s in `bufWrite` are unnecessary, though. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:13 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: #9164 -------------------------------------+------------------------------------ Changes (by nomeata): * related: => #9164 Comment: Discussion about `castPtr` moved to #9164. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:14 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: #9164 -------------------------------------+------------------------------------ Comment (by nomeata): I agree with rwbarton: The `Ptr`’s parameter is, in a sense, a convenience for the user. The API could just use `Ptr` without an argument, and users could use `Tagged a Ptr` if they want to track types. By that reasoning, `Ptr`’s argument should be phantom, as nothing in `Foreign.Ptr` takes `a` into account. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:15 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: #9164 -------------------------------------+------------------------------------ Comment (by simonpj): OK so * I think we have consensus that `Ptr` should have phantom role. Joachim, could you push that through? * Am am not sure what the story is on `FunPtr`. Maybe it doesn't matter for now; but if we leave it as it is, could we add a comment to the role signature saying that it's a conservative choice, and pointing to this ticket? It'd be nice to get this done and closed. Thanks Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:16 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role
-------------------------------------+------------------------------------
Reporter: simonpj | Owner:
Type: bug | Status: new
Priority: normal | Milestone:
Component: Compiler | Version: 7.8.2
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture: Unknown/Multiple
Type of failure: None/Unknown | Difficulty: Unknown
Test Case: | Blocked By:
Blocking: | Related Tickets: #9164
-------------------------------------+------------------------------------
Comment (by Joachim Breitner

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: #9164 -------------------------------------+------------------------------------ Changes (by nomeata): * status: new => closed * resolution: => fixed Comment: ✓ -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:18 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: #9164 -------------------------------------+------------------------------------ Comment (by simonmar): I think it would be good to do this for `FunPtr` too, if it's not too hard. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:19 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: #9164 -------------------------------------+------------------------------------ Changes (by nomeata): * status: closed => new * resolution: fixed => Comment: I had to revert the patch, adding `NoImplicitPrelude` to `Data.Coerce` makes the build system trip over. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:20 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: #9164 -------------------------------------+------------------------------------ Changes (by simonpj): * cc: core-libraries-committee@… (added) Comment: FWIW I'd also be happy to treat `FunPtr` just like `Ptr` (ie phantom). I'll add the core-libraries-committee in cc Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:21 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role
-------------------------------------+------------------------------------
Reporter: simonpj | Owner:
Type: bug | Status: new
Priority: normal | Milestone:
Component: Compiler | Version: 7.8.2
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture: Unknown/Multiple
Type of failure: None/Unknown | Difficulty: Unknown
Test Case: | Blocked By:
Blocking: | Related Tickets: #9164
-------------------------------------+------------------------------------
Comment (by Joachim Breitner

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------ Reporter: simonpj | Owner: goldfire Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: #9164 -------------------------------------+------------------------------------ Changes (by goldfire): * owner: => goldfire Comment: As noted previously, `FunPtr`'s roles are a little fiddlier. I'll take this one, and add a bunch of comments while I'm at it. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:23 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role ------------------------------------------------+-------------------------- Reporter: simonpj | Owner: Type: bug | goldfire Priority: normal | Status: new Component: Compiler | Milestone: Resolution: | Version: 7.8.2 Operating System: Unknown/Multiple | Keywords: Type of failure: None/Unknown | Architecture: Test Case: roles/should_compile/Roles2 | Unknown/Multiple Blocking: | Difficulty: | Unknown | Blocked By: | Related Tickets: #9164 ------------------------------------------------+-------------------------- Changes (by goldfire): * testcase: => roles/should_compile/Roles2 Comment: Patch on the way... -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:24 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role
------------------------------------------------+--------------------------
Reporter: simonpj | Owner:
Type: bug | goldfire
Priority: normal | Status: new
Component: Compiler | Milestone:
Resolution: | Version: 7.8.2
Operating System: Unknown/Multiple | Keywords:
Type of failure: None/Unknown | Architecture:
Test Case: roles/should_compile/Roles2 | Unknown/Multiple
Blocking: | Difficulty:
| Unknown
| Blocked By:
| Related Tickets: #9164
------------------------------------------------+--------------------------
Comment (by Richard Eisenberg

#9163: Ptr should have a phantom role ------------------------------------------------+-------------------------- Reporter: simonpj | Owner: Type: bug | goldfire Priority: normal | Status: merge Component: Compiler | Milestone: 7.8.4 Resolution: | Version: 7.8.2 Operating System: Unknown/Multiple | Keywords: Type of failure: None/Unknown | Architecture: Test Case: roles/should_compile/Roles2 | Unknown/Multiple Blocking: | Difficulty: | Unknown | Blocked By: | Related Tickets: #9164 ------------------------------------------------+-------------------------- Changes (by goldfire): * status: new => merge * milestone: => 7.8.4 Comment: I believe this closes this ticket. Suggesting merging because of related performance improvement, but it's not critical. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:26 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role ------------------------------------------------+-------------------------- Reporter: simonpj | Owner: Type: bug | Status: new Priority: normal | Milestone: 7.8.4 Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: None/Unknown | Unknown/Multiple Test Case: roles/should_compile/Roles2 | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: #9164 ------------------------------------------------+-------------------------- Changes (by goldfire): * owner: goldfire => * status: merge => new Comment: Can't disown a "merge" ticket. Reopening to disown. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:27 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role ------------------------------------------------+-------------------------- Reporter: simonpj | Owner: Type: bug | Status: merge Priority: normal | Milestone: 7.8.4 Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: None/Unknown | Unknown/Multiple Test Case: roles/should_compile/Roles2 | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: #9164 ------------------------------------------------+-------------------------- Changes (by goldfire): * status: new => merge -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:28 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role ------------------------------------------------+-------------------------- Reporter: simonpj | Owner: Type: bug | Status: closed Priority: normal | Milestone: 7.10.1 Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: None/Unknown | Unknown/Multiple Test Case: roles/should_compile/Roles2 | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: #9164 ------------------------------------------------+-------------------------- Changes (by thoughtpolice): * status: merge => closed * resolution: => fixed * milestone: 7.8.4 => 7.10.1 Comment: I'm leaving these as is out of 7.8.3; I don't think they're critical, and some of the refactorings in `base` to `Data.Coerce` mean the original patches don't quite apply cleanly. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:29 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 7.10.1 Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | roles/should_compile/Roles2 Blocked By: | Blocking: Related Tickets: #9164 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dominic): {{{ *Numeric.Sundials.CVode.ODE> (coerce ((V.fromList [1,2]) :: Vector CInt)) :: Vector Int [8589934593,4426664329] }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:30 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 7.10.1 Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | roles/should_compile/Roles2 Blocked By: | Blocking: Related Tickets: #9164 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): dominic, what is comment:30 trying to say? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:31 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 7.10.1 Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | roles/should_compile/Roles2 Blocked By: | Blocking: Related Tickets: #9164 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dominic): Replying to [comment:31 simonpj]:
dominic, what is comment:30 trying to say?
I think there is bug. This should be a type error. Here's a PR: https://phabricator.haskell.org/D4941. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:32 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 7.10.1 Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | roles/should_compile/Roles2 Blocked By: | Blocking: Related Tickets: #9164 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dominic): BTW you can't coerce a CInt to an Int so why should you be able to coerce a Vector of CInts to a Vector of Int? {{{ Prelude Data.Coerce Foreign.C.Types Data.Int> coerce (0 :: CInt) :: Int <interactive>:5:1: error: • Couldn't match representation of type 'Int32' with that of 'Int' arising from a use of 'coerce' • In the expression: coerce (0 :: CInt) :: Int In an equation for 'it': it = coerce (0 :: CInt) :: Int }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:33 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 7.10.1 Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | roles/should_compile/Roles2 Blocked By: | Blocking: Related Tickets: #9164 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by RyanGlScott): I think there's a bit of a tension here between two different camps who want `Ptr`'s role signature to be phantom or representational for different purposes: 1. Some have argued that it should be phantom because that allows one to implement `castPtr` (a quite fundamental operation) efficiently via `coerce`. 2. Others have argued that it should be representational to avoid coercing between `Ptr`s whose underlying types have different representations. Personally, I find myself more aligned towards the (1) camp, for the simple reason that `Ptr`s are not intended to be a safe abstraction. There are many ways you can cause a Haskell program to segfault through reckless use of `Ptr`s (even without `castPtr`), so changing its role signature isn't going to change things in this regard. In light of this, I'm inclined to believe that the proper fix to dominic's problem is to give `Vector` a role signature of representational, as `Vector` is what is intended to be a safe abstraction. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:34 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 7.10.1 Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | roles/should_compile/Roles2 Blocked By: | Blocking: Related Tickets: #9164 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Is this really a discussion for the libraries list? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:35 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 7.10.1 Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | roles/should_compile/Roles2 Blocked By: | Blocking: Related Tickets: #9164 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by RyanGlScott): If dominic feels strongly about this, then yes, I'd encourage that they take this issue to the libraries list. I posted comment:34 not necessarily to start the debate afresh, but to give an overview of how things came to be, and to give my (editorialized) view on a simple way to resolve this issue. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:36 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 7.10.1 Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | roles/should_compile/Roles2 Blocked By: | Blocking: Related Tickets: #9164 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dominic): I do feel strongly. From https://hackage.haskell.org/package/base-4.8.2.0/docs/Data-Coerce.html: "The function coerce allows you to safely convert between values of types". If the user has to know that the package they are using uses Ptr and thus avoid coerce then that seems to break the principle of abstraction. At least change the claim that coerce is type safe. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:37 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 7.10.1 Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | roles/should_compile/Roles2 Blocked By: | Blocking: Related Tickets: #9164 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dfeuer): goldfire, this gets back to what we discussed in Philly. What we *really* want is for `Ptr` to be defined with a phantom argument, but exported with a representational one. We can work around the trouble today using a newtype: {{{#!hs data Addr = Addr Addr# newtype Ptr a = Ptr_ Addr type role Ptr representational pattern Ptr :: Addr# -> Ptr a pattern Ptr addr# = Ptr_ (Addr addr#) castPtr :: Ptr a -> Ptr b castPtr = coerce ptrCoercion :: Coercion (Ptr a) (Ptr b) ptrCoercion = Coercion }}} The only trouble I see is that the `Generic` representation will change. I kind of doubt anyone's relying on the details of that representation. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:38 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 7.10.1 Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | roles/should_compile/Roles2 Blocked By: | Blocking: Related Tickets: #9164 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by dfeuer): * cc: dfeuer (added) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:39 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 7.10.1 Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | roles/should_compile/Roles2 Blocked By: | Blocking: Related Tickets: #9164 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by goldfire): comment:38 may indeed work. But `Ptr` is baked into the compiler in places, so you'll have to tread carefully if you update it. Specifically, look at `normaliseFfiType'`. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:40 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#9163: Ptr should have a phantom role -------------------------------------+------------------------------------- Reporter: simonpj | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 7.10.1 Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: | roles/should_compile/Roles2 Blocked By: | Blocking: Related Tickets: #9164 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by dominic): My concerns are fixed here: https://github.com/haskell/vector/pull/224 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9163#comment:41 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC