Proposal: Move primitive-Data.Primitive.Addr API into base

Fri Oct 26 17:09:09 UTC 2018

https://phabricator.haskell.org/D5268

On Fri, Oct 26, 2018 at 1:08 PM Carter Schonwald <carter.schonwald at gmail.com>
wrote:

> On the flip side we could just add data Addr ... to foreign .ptr or a
> child / sibling module like System.Mem.Address and defer the associated
> cleanup
>
> I kinda like the idea of System.Mem.Address,
>
> i put a strawman change set on phab https://phabricator.haskell.org/D5268
>
> On Fri, Oct 26, 2018 at 12:23 PM Carter Schonwald <
> carter.schonwald at gmail.com> wrote:
>
>> granted most of the Addr# sites dont matter
>>
>> a related issue is what operations WOULD be added for Addr?
>>
>> maybe just memcopy friends plus address arithmetic, plus a mapper from/to
>> Ptr a?
>>
>> does it simply become a byte size step sibling to Ptr a that put in
>> Foreign.Ptr?
>>
>> On Fri, Oct 26, 2018 at 12:18 PM Carter Schonwald <
>> carter.schonwald at gmail.com> wrote:
>>
>>> Before we bike shed these details, let’s evaluate / document the pieces
>>> of base that should also be moved to using Addr.  Though that module where
>>>
>>> 1) What exported types / data definitions / functions will have changes ?
>>>
>>> 2) what internal apis need to change?
>>>
>>> as a lower bound estimate of work surface area, i grepped for "Ptr ()"
>>> and "Addr#" in a fresh recursive clone of ghc at
>>> commit 503514b94f8dc7bd9eab5392206649aee45f140b,
>>> limiting my search to .hs and .hsc files
>>>
>>> THERES A LOT of uses of Ptr (), and its siblings StablePtr () and
>>> RemotePtr () ,   The latter two dont have a simple Addr# underneath,
>>>
>>> heres the list of files that match that search   rg 'Ptr \(\)' --type
>>> haskell -c
>>>
>>> ghc/GHCi/Leak.hs:1
>>> utils/hsc2hs/Main.hs:1
>>> utils/runghc/Main.hs:1
>>> compiler/ghci/GHCi.hs:4
>>> compiler/ghci/ByteCodeLink.hs:3
>>> compiler/ghci/ByteCodeGen.hs:4
>>> compiler/ghci/ByteCodeTypes.hs:1
>>> compiler/ghci/ByteCodeAsm.hs:2
>>> compiler/main/HscTypes.hs:1
>>> libraries/ghci/GHCi/ObjLink.hs:3
>>> libraries/ghci/GHCi/StaticPtrTable.hs:1
>>> libraries/ghci/GHCi/ResolvedBCO.hs:1
>>> libraries/ghci/GHCi/RemoteTypes.hs:2
>>> libraries/ghci/GHCi/Run.hs:2
>>> libraries/ghci/GHCi/Message.hs:5
>>> libraries/base/GHC/Stable.hs:2
>>> libraries/base/GHC/StaticPtr.hs:1
>>> libraries/base/GHC/Generics.hs:2
>>> libraries/base/GHC/TopHandler.hs:1
>>> libraries/base/GHC/Windows.hs:1
>>> libraries/base/tests/dynamic002.hs:2
>>> utils/haddock/haddock-api/src/Haddock.hs:1
>>> libraries/array/Data/Array/IO.hs:2
>>> libraries/base/Foreign/Marshal/Pool.hs:1
>>> libraries/bytestring/Data/ByteString/Builder/Extra.hs:1
>>> libraries/base/GHC/IO/Handle/Text.hs:1
>>> testsuite/tests/rts/T7160.hs:1
>>> testsuite/tests/rts/overflow3.hs:1
>>> testsuite/tests/rts/overflow2.hs:1
>>> testsuite/tests/rts/T5250.hs:1
>>> testsuite/tests/rts/T10296b.hs:2
>>> testsuite/tests/th/T3319.hs:1
>>> testsuite/tests/th/TH_foreignInterruptible.hs:1
>>> testsuite/tests/th/TH_foreignCallingConventions.hs:1
>>> libraries/Cabal/cabal-install/Distribution/Client/Win32SelfUpgrade.hs:1
>>> libraries/Cabal/Cabal/Distribution/Simple/InstallDirs.hs:2
>>> testsuite/tests/llvm/should_compile/T5486.hs:2
>>> testsuite/tests/codeGen/should_run/cgrun070.hs:5
>>> testsuite/tests/codeGen/should_run/cgrun026.hs:3
>>> testsuite/tests/primops/should_run/T4442.hs:1
>>> testsuite/tests/ffi/should_fail/T7243.hs:1
>>> testsuite/tests/ffi/should_fail/T3066.hs:1
>>> testsuite/tests/ffi/should_compile/cc009.hs:1
>>> testsuite/tests/ffi/should_run/ffi021.hs:1
>>> testsuite/tests/typecheck/should_compile/T7408.hs:1
>>>
>>> libraries/Cabal/cabal-install/Distribution/Client/Compat/ExecutablePath.hs:1
>>> libraries/Cabal/Cabal/Distribution/Simple/Build/PathsModule.hs:1
>>> libraries/time/lib/Data/Time/Clock/Internal/CTimeval.hs:1
>>> testsuite/tests/rename/should_compile/T3103/Foreign/Ptr.hs:1
>>>
>>> likewise the locations where Addr# comes up are as follows (some of
>>> these perhaps should be an Addr? )
>>>
>>> $ rg 'Addr#' --type haskell -c
>>> utils/genprimopcode/Main.hs:2
>>> ghc/GHCi/Leak.hs:2
>>> compiler/cmm/CmmMachOp.hs:1
>>> compiler/cmm/CmmUtils.hs:1
>>> compiler/specialise/Rules.hs:2
>>> compiler/basicTypes/MkId.hs:2
>>> compiler/basicTypes/Literal.hs:6
>>> compiler/basicTypes/Unique.hs:2
>>> compiler/utils/StringBuffer.hs:1
>>> compiler/utils/FastString.hs:4
>>> compiler/utils/Encoding.hs:9
>>> compiler/codeGen/StgCmmPrim.hs:3
>>> compiler/coreSyn/MkCore.hs:5
>>> compiler/coreSyn/CoreLint.hs:2
>>> compiler/coreSyn/CoreSyn.hs:4
>>> compiler/coreSyn/CorePrep.hs:1
>>> compiler/stranal/WwLib.hs:1
>>> compiler/simplCore/CSE.hs:1
>>> compiler/simplCore/Simplify.hs:2
>>> compiler/deSugar/DsExpr.hs:1
>>> compiler/typecheck/TcGenDeriv.hs:5
>>> compiler/prelude/ForeignCall.hs:1
>>> compiler/prelude/PrelNames.hs:1
>>> compiler/prelude/TysPrim.hs:1
>>> compiler/ghci/ByteCodeGen.hs:8
>>> libraries/bytestring/tests/revcomp.hs:1
>>> libraries/bytestring/Data/ByteString.hs:5
>>> libraries/ghc-heap/tests/heap_all.hs:1
>>> libraries/ghc-prim/GHC/CString.hs:17
>>> libraries/ghc-prim/GHC/Classes.hs:1
>>> libraries/ghc-prim/GHC/Types.hs:3
>>> libraries/ghci/GHCi/BinaryArray.hs:1
>>> libraries/ghci/GHCi/CreateBCO.hs:5
>>> libraries/base/codepages/MakeTable.hs:1
>>> libraries/base/GHC/Ptr.hs:8
>>> libraries/base/GHC/Generics.hs:5
>>> libraries/base/GHC/Storable.hs:32
>>> libraries/base/GHC/ForeignPtr.hs:6
>>> libraries/base/Foreign/Ptr.hs:2
>>> libraries/bytestring/Data/ByteString/Unsafe.hs:6
>>> libraries/bytestring/Data/ByteString/Internal.hs:7
>>> libraries/ghc-heap/GHC/Exts/Heap.hs:1
>>> libraries/text/Data/Text/Show.hs:2
>>> libraries/text/Data/Text/Lazy.hs:2
>>> libraries/containers/Data/IntSet/Internal.hs:4
>>> libraries/ghc-compact/GHC/Compact/Serialized.hs:11
>>> libraries/base/GHC/IO/Exception.hs:1
>>> libraries/base/Data/Typeable/Internal.hs:3
>>> libraries/base/Control/Exception/Base.hs:1
>>> utils/haddock/html-test/src/Ticket112.hs:1
>>> testsuite/tests/rts/T8242.hs:19
>>> testsuite/tests/ghci.debugger/HappyTest.hs:3
>>> testsuite/tests/arityanal/Main.hs:3
>>> testsuite/tests/th/TH_StringPrimL.hs:2
>>> testsuite/tests/th/T1830_3a.hs:2
>>> libraries/template-haskell/Language/Haskell/TH/Syntax.hs:1
>>> libraries/bytestring/Data/ByteString/Short/Internal.hs:16
>>> libraries/integer-gmp/src/GHC/Integer/Type.hs:11
>>> libraries/base/GHC/IO/Encoding/CodePage.hs:3
>>> libraries/Cabal/Cabal/Distribution/Parsec/Lexer.hs:9
>>> testsuite/tests/generics/GEq/GEq1A.hs:1
>>> testsuite/tests/generics/GEq/GEq1.hs:2
>>> testsuite/tests/codeGen/should_compile/cg008.hs:1
>>> testsuite/tests/codeGen/should_run/cgrun070.hs:4
>>> testsuite/tests/codeGen/should_run/T9577.hs:1
>>> testsuite/tests/codeGen/should_run/cgrun066.hs:1
>>> testsuite/tests/codeGen/should_run/cgrun026.hs:2
>>> testsuite/tests/codeGen/should_run/T2080.hs:2
>>> testsuite/tests/codeGen/should_run/T9577_A.hs:1
>>> testsuite/tests/primops/should_run/T4442.hs:4
>>> testsuite/tests/simplCore/should_compile/T13367.hs:1
>>> testsuite/tests/simplCore/should_compile/T13155.hs:2
>>> testsuite/tests/simplCore/should_compile/T14779a.hs:1
>>> testsuite/tests/simplCore/should_compile/T14779b.hs:2
>>> testsuite/tests/simplCore/should_compile/spec001.hs:1
>>> testsuite/tests/lib/integer/integerGmpInternals.hs:9
>>> testsuite/tests/perf/compiler/T14683.hs:3
>>> testsuite/tests/ghci/should_run/T2589.hs:1
>>> libraries/integer-gmp/src/GHC/Integer/GMP/Internals.hs:7
>>> libraries/text/Data/Text/Internal/Fusion/Common.hs:3
>>> libraries/base/GHC/IO/Encoding/CodePage/Table.hs:1
>>>
>>>
>>>
>>> On Fri, Oct 26, 2018 at 11:01 AM Daniel Cartwright <
>>> chessai1996 at gmail.com> wrote:
>>>
>>>> Also, where in base should Addr go?
>>>>
>>>> On Fri, Oct 26, 2018 at 10:15 AM Daniel Cartwright <
>>>> chessai1996 at gmail.com> wrote:
>>>>
>>>>> I would, yeah. Should the next version of `primitive` just re-export
>>>>> `Addr` from base in `Data.Primitive.Addr`?
>>>>>
>>>>> On Fri, Oct 26, 2018 at 10:14 AM Carter Schonwald <
>>>>> carter.schonwald at gmail.com> wrote:
>>>>>
>>>>>> Daniel or David, would either of you be interested/willing to do the
>>>>>> legwork on this patch wise (subject to sanity checking whats the impact
>>>>>> surface area?)
>>>>>>
>>>>>> On Fri, Oct 26, 2018 at 12:04 AM Carter Schonwald <
>>>>>> carter.schonwald at gmail.com> wrote:
>>>>>>
>>>>>>> Cool. What apis in base should move to use Addr from fake pointer ?
>>>>>>> Cause just adding it in isolation seems lame!
>>>>>>>
>>>>>>> I guess I’m just trying to say “what would the impact on base, if
>>>>>>> every fake Ptr was moved to be Addr?” Because it’s not something which
>>>>>>> should be done by halves.
>>>>>>>
>>>>>>> 1) what apis would be changed?
>>>>>>>
>>>>>>> 2) what internal things would be changed albeit not Api visible
>>>>>>>
>>>>>>> 3). Address arithmetic for Addr is very different from addres
>>>>>>> arithmetic on larger than word8 size storable values, is there anything
>>>>>>> which would move representations where the change to byte indexed would
>>>>>>> change the calculations ?
>>>>>>>
>>>>>>>
>>>>>>> If we can layout what the impact / scope of changes needed for these
>>>>>>> and other considerations, I guess I’d be all for it. Assuming there’s the
>>>>>>> corresponding interest in executing changes. (I just have this worry in my
>>>>>>> head that it might be one of those icebergs.. like the split base effort )
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Oct 25, 2018 at 11:33 PM David Feuer <david.feuer at gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> On Thu, Oct 25, 2018, 11:28 PM Carter Schonwald <
>>>>>>>> carter.schonwald at gmail.com> wrote:
>>>>>>>>
>>>>>>>>> Pretending we’re talking about this with storable as our example
>>>>>>>>> semantics :
>>>>>>>>>
>>>>>>>>> Ptr Void
>>>>>>>>> Corresponds to a memory address you don’t want to read or write to.
>>>>>>>>>
>>>>>>>>
>>>>>>>> This is not so easy to think about (there's not much utility in
>>>>>>>> such a type). Perhaps Ptr Void is a reasonable type for pointers known to
>>>>>>>> be null or outside the address space, but I'm not sure.
>>>>>>>>
>>>>>>>>
>>>>>>>>> Ptr () corresponds to a memory location that’s pretty boring to
>>>>>>>>> read / write to
>>>>>>>>>
>>>>>>>>
>>>>>>>> Yes! In other words, it's *not* what people should be using for a
>>>>>>>> pointer to an arbitrary non-Haskell address. That's precisely what Addr is
>>>>>>>> for.
>>>>>>>>
>>>>>>>>
>>>>>>>>> On Thu, Oct 25, 2018 at 11:22 PM Carter Schonwald <
>>>>>>>>> carter.schonwald at gmail.com> wrote:
>>>>>>>>>
>>>>>>>>>> ... when is a valid pointer not going to point at byte
>>>>>>>>>> addressable memory on memory architectures ghc can support or target?
>>>>>>>>>>
>>>>>>>>>> On Thu, Oct 25, 2018 at 8:27 PM Daniel Cartwright <
>>>>>>>>>> chessai1996 at gmail.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Now, one could argue that `Ptr ()` isn't a lie, it sort of reads
>>>>>>>>>>> like C's void pointer. But surely something like `Ptr Word8` is a lie, when
>>>>>>>>>>> it is not actually a Ptr to Word8 values.
>>>>>>>>>>>
>>>>>>>>>>> On Thu, Oct 25, 2018 at 8:11 PM Daniel Cartwright <
>>>>>>>>>>> chessai1996 at gmail.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> yes, only the type and its instances should be moved as far as
>>>>>>>>>>>> i'm aware.
>>>>>>>>>>>>
>>>>>>>>>>>> Also, it's more than just base.
>>>>>>>>>>>>
>>>>>>>>>>>> this Ptr is a lie:
>>>>>>>>>>>> https://hackage.haskell.org/package/ghc-compact-0.1.0.0/docs/GHC-Compact-Serialized.html#t:SerializedCompact
>>>>>>>>>>>> these Ptrs are lies:
>>>>>>>>>>>> https://hackage.haskell.org/package/base-4.12.0.0/docs/GHC-IO-Handle.html
>>>>>>>>>>>> in GHC.Stats, the foreign import "getRTSStats" has `Ptr () ->
>>>>>>>>>>>> IO ()`, this Ptr () is also a lie
>>>>>>>>>>>>
>>>>>>>>>>>> These are just off the top of my head, there are more
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Thu, Oct 25, 2018 at 6:46 PM Carter Schonwald <
>>>>>>>>>>>> carter.schonwald at gmail.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> hrmm, what are the pieces of base that are using Ptr when they
>>>>>>>>>>>>> really should be using Addr? This would help me understand what would be
>>>>>>>>>>>>> made better in base :)
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Thu, Oct 25, 2018 at 6:19 PM David Feuer <
>>>>>>>>>>>>> david.feuer at gmail.com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> We shouldn't really need to move anything into base except
>>>>>>>>>>>>>> Addr and its base instances.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Oct 25, 2018 5:59 PM, "Carter Schonwald" <
>>>>>>>>>>>>>> carter.schonwald at gmail.com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Indeed.  The monad transformer instances for primmonad need
>>>>>>>>>>>>>> to live in primmonad OR transformers to avoid orphans.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Either way, unless transformers moves into base (unlikely),
>>>>>>>>>>>>>> no way anything using prim monad will.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Thu, Oct 25, 2018 at 3:34 PM Andrew Martin <
>>>>>>>>>>>>>> andrew.thaddeus at gmail.com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I like the idea of moving the type Addr into base. But we
>>>>>>>>>>>>>>> cannot move the entire module since it has functions that talk about
>>>>>>>>>>>>>>> PrimMonad, and we definitely don't want to move that into base.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Thu, Oct 25, 2018 at 11:25 AM Daniel Cartwright <
>>>>>>>>>>>>>>> chessai1996 at gmail.com> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Motivation: There are a lot of places in base where 'Ptr a'
>>>>>>>>>>>>>>>> is used in place of 'Addr', because in base there is no 'Addr', only
>>>>>>>>>>>>>>>> 'Addr#'. The problem lies in the fact that many of these uses of 'Ptr a'
>>>>>>>>>>>>>>>> are lying; the 'a' value is meaningless. Authors of functions therein have
>>>>>>>>>>>>>>>> used things like 'Ptr ()', 'Ptr Word8', 'Ptr a', but these types do not
>>>>>>>>>>>>>>>> mean what they say they mean - they're just Addr. There are probably other
>>>>>>>>>>>>>>>> motivations for this that I can't think of off the top of my head right now.
>>>>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>>>>> Libraries mailing list
>>>>>>>>>>>>>>>> Libraries at haskell.org
>>>>>>>>>>>>>>>> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>> -Andrew Thaddeus Martin
>>>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>>>> Libraries mailing list
>>>>>>>>>>>>>>> Libraries at haskell.org
>>>>>>>>>>>>>>> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>>> Libraries mailing list
>>>>>>>>>>>>>> Libraries at haskell.org
>>>>>>>>>>>>>> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>> Libraries mailing list
>>>>>>>>>>>>> Libraries at haskell.org
>>>>>>>>>>>>> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
>>>>>>>>>>>>>
>>>>>>>>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/libraries/attachments/20181026/71cab62e/attachment-0001.html>