Potential Bug: usage of unsafePerformIO? about reactive-banana HOT 5 CLOSED

That's good point.

The uses of unsafePerformIO in the module PulseLatch0 are both harmless and unnecessary; the only trouble is let-floating, which I have carefully avoided.

In contrast, the use of unsafePerformIO for sharing in the Cached module is more serious, and that's why it also has a NOINLINE pragma.

Still, the ramifications of unsafePerformIO are a little difficult to survey, so I should probably err on the side of caution and add the various flags.

from reactive-banana.

I think this issue could be closed after we determine what to do about this one:

-- | Return a 'Latch' that has a constant value
pureL :: a -> Latch a
pureL a = unsafePerformIO $ Ref.new $ Latch
    { _seenL  = beginning
    , _valueL = a
    , _evalL  = return a
    }

It seems to me that it would be fine for pureL to be inlined, but it might be better to add NOINLINE anyway.

from reactive-banana.

It seems to me that it would be fine for pureL to be inlined, but it might be better to add NOINLINE anyway.

I agree.

Ideally, for reasons of efficiency, pureL would get its own constructor in Latch, but slapping NOINLINE on this and calling it a day sounds good to me.

from reactive-banana.

@HeinrichApfelmus Interesting, do you mean this?

data Latch a
  = PureL a
  | NotPureL {- what it is now -}

If so, I can make a new ticket to track that.

from reactive-banana.

do you mean this?

Yes, indeed.

If so, I can make a new ticket to track that.

Yes, please. That said, there are some performance issues with Latch that I need to think carefully about, and I'm not sure if the definition of Latch that you gave will survive. Then again, a constructor for PureL is highly likely.

from reactive-banana.