I have casually looked through the code and I saw nothing against it. As for the technique, I have to say it's a clever trick :). I normally would have opposition of principle against unsafePerformIO, but you could argue that serializable dynamics should be supported natively by GHC anyway. That said, I remember that some version of Yi played badly with unsafePerformIO... Did you check that dynamically executed code works? If so I see no reason to delay application of the patch.
Cheers, JP. On Fri, May 20, 2011 at 6:34 AM, Reiner Pope <reiner.p...@gmail.com> wrote: > Hi all, > > I've written a patch which I would like reviewed before pushing. > > You can view the changes at [1]. The patch is a rewrite of the Yi.Dynamic > module to support serialisation on Dynamics (which means that things like > minibuffer history are preserved across 'reloadConfigE'). The module > Yi.Dynamic contains the interesting changes; most other changes are just the > necessary instances of Binary for the modules which were using Yi.Dynamic. > > I'll briefly explain the new implementation. We might try to implement > serialisable dynamics as > > data Dynamic = forall a. (Binary a, Typeable a) => Dynamic a > > Serialisation is easy, but deserialisation is not: when we're deserialising, > all we can read in is the TypeRep and the serialised data (a ByteString). > How can we possibly find the correct 'Binary a' instance from that? > > So, we add another constructor: > > data Dynamic = forall a. (Binary a, Typeable a) => Dynamic a > | Serial TypeRep ByteString > > The trick is that we get the Binary instance when we read from the dynamic: > > fromDynamic :: (Typeable a, Binary a) => Dynamic -> Maybe a > fromDynamic (Serial tr bs) = if<check the types> then Just (decode bs) else > Nothing > > This is all good, except we run 'decode' every time we read from the > Dynamic. > > I decided to avoid this unnecessary work by wrapping the Dynamic in an > IORef, and changing the representation from Serial to Dynamic once we have > run fromDynamic. Everything is then wrapped in unsafePerformIO, to present a > pure interface. > > What do you think of this decision? The exported API is genuinely pure, but > the implementation is not. > > Cheers, > Reiner > > [1] > https://github.com/reinerp/yi/commit/087263a1d1d7374d3c7fc1195c0d322e488b36e1 > > -- > Yi development mailing list > yi-devel@googlegroups.com > http://groups.google.com/group/yi-devel -- Yi development mailing list yi-devel@googlegroups.com http://groups.google.com/group/yi-devel
