
On Fri, 24 Dec 2010, Eugene Kirpichov wrote:
I've released storable-endian 0.2.0, which does not use TH and bases on your suggestion (though it has a bit of boilerplate because of abandoning TH, but I don't think that's critical).
Great, this should make it also usable on JHC (I have not tested so far). However I'm still concerned about the use of so much Unsafe functions. You use unsafePeekOff and then wrap it in IO monad, again. Could you also stay in IO monad and omit unsafePerformIO? You use unsafeCoerce for mapping from (IO Word16) to (IO Int16) - how about just (fmap fromIntegral)? Maybe you succeed to find out machine endianess at compile time. Then you could use native memory access if the user requested endianess is the one of the machine. If you achieve this, you could implement peek and poke of Float and Double by just reversing the byte order into an 'alloca'd memory area and peek from and poke into this one with a native peek or poke, respectively. That is you would not need any unsafeCoerce anymore. Two suggestions on style: I find the class names with "Has" prefix not so nice. "Has" the type Double a Big Endian or a Little Endian? How about class names LittleEndianStorable or LittleEndianOrder? I think I like LittleEndianStorable more, since it makes clear, that this is a class similar to Storable but specialised to Little Endian byte order. I'd like to have GHC specific code in a separate module instead of preprocessor instructions, that become quickly confusing if you add support for more compilers (say JHC). You may put GHC specific code into a private module, say Data.Storable.Endian.Private, and create two instances of this module in two directories, say 'generic' and 'ghc'. Then in storable-endian.cabal you write: -- for Data.Storable.Endian Hs-Source-Dirs: src -- for Data.Storable.Endian.Private If impl(ghc) Hs-Source-Dirs: ghc Else Hs-Source-Dirs: generic ... and thank you very much for creating this package!