reactive-banana icon indicating copy to clipboard operation
reactive-banana copied to clipboard

A version of compile that returns a side value

Open ChristopherKing42 opened this issue 6 years ago • 12 comments
trafficstars

I think it would be useful if there was a version of compile that in addition to compiling the network allowed MomentIO to return a side value:

compile' :: MomentIO a -> IO (a, EventNetwork)

This allows you to, for example, extract the initial values of behaviors. Looking at the source code, it appears that there already is code to do the above, it just isn't exported in a nice way.

ChristopherKing42 avatar Sep 10 '19 22:09 ChristopherKing42

I'd really like to have this, but am stuck in the age old problem of naming. I think changing compile would ultimately be the nicest, but it's just too big a breaking change to do without a deprecation cycle, which would itself the require another deprecation cycle to switch /back/ to compile. So I think it's best we just have both.

I don't like using a prime, as I associate that was strictness.

Any preferences for names? compileAndReturn maybe?

ocharles avatar Sep 08 '22 22:09 ocharles

Cc @HeinrichApfelmus and @mitchellwrosen

ocharles avatar Sep 08 '22 22:09 ocharles

Hmm... no strong feelings either way :sweat_smile:.

In defense of the status quo, I've only ever called compile at type MomentIO (), so I'd never have preferred this variant which would hand me back a ((), EventNetwork). And it is possible to smuggle a return value out of course, with an MVar.

Having compile and compileAndReturn/compile'/compile2 would be ok; I don't have any good naming suggestions. I'm also not opposed to just changing the type of compile. Since reactive-banana is mostly used in applications (and not wrapped much by other libraries), and each application probably only has a single call to compile, the breakage would be pretty minimal.

Overall, it's unclear to me whether it'd be better to have:

  1. The current API, where the only way to get a non-() return value out of compile looks like:
var <- newEmptyMVar
network <- compile (theNetwork >> liftIO (putMVar var thing))
thing <- readMVar var
actuate network
  1. Only this proposed version of compile, where code that used to look like
network <- compile theNetwork
actuate network

would instead look like

((), network) <- compile theNetwork
actuate network
  1. Both versions in the same API.

Reflecting on it a bit... I think I weakly prefer (1), followed by (2), then (3)

mitchellwrosen avatar Sep 09 '22 01:09 mitchellwrosen

The reason I'm interested in this is because it simplifies the following pattern:

setup :: ... -> IO Callbacks
setup ... = do
  (aAddHandler, fireA) <- newAddHandler
  (bAddHandler, fireB) <- newAddHandler
  ...
  (zAddHandler, fireZ) <- newAddHandler

  actuate =<< compile do
    onA <- fromAddHandler
    onB <- fromAddHandler
    ...
    onZ <- fromAddHandler
    ...

  return Callbacks{ fireA, fireB, ..., fireZ }

to

setup :: ... -> IO Callbacks
setup ... = do 
  (eventNetwork, callbacks) <- compile do
    (onA, fireA) <- newEvent
    (onB, fireB) <- newEvent
    ...
    (onZ, fireZ) <- newEvent
    ...
    return Callbacks{..}

  actuate eventNetwork
  return callbacks 

Smuggling that out with an MVar is certainly an option, but why jump through that hoop?

I'm also fairly liberal with breakage (and don't really think reactive-banana has a huge usage at the moment sadly), and would take a compile type change.

@HeinrichApfelmus Maybe you want to be a tie-breaker :smile:

ocharles avatar Sep 09 '22 08:09 ocharles

Good point, I like that pattern. Ok, I switch to (2), then (3), then (1).

mitchellwrosen avatar Sep 09 '22 13:09 mitchellwrosen

I wasn't sure how other people would feel about it, but if it's on the table option 2 is my personal favorite.

ChristopherKing42 avatar Sep 11 '22 02:09 ChristopherKing42

As far as the aesthetics of the interface goes, I would prefer (2) and (3) as well. However, there is catch, and this catch is the main reason why I made it difficult to get stuff out of a compile, and strongly prefer (1).

The catch is that it now also becomes possible to return Event or Behavior from the compile and use it in another network — but, the semantics of this is completely undefined. 😱 Example:

setup :: … -> IO ()
setup … = do
    (networkA, eA) <- compile' $ do
        (e, fire) <- newEvent
        …
        return e

    (networkB, _) <- compile' $ do
        (eB, fireB) <- newEvent
        let eBoom = unionWith (+) eA eB -- 😱
        reactimate $ print <$> eBoom    -- 😱😱😱
        return ()

    actuate networkB
    actuate networkA

In earlier versions of reactive-banana, I actually used a type parameter to prevent this from happening, but eventually decided that this parameter made so much noise in the types that we would be better of with disallowing return values in compile.

So, unfortunately, I'd like to stick to (1). (But happy to add an explanation of this somewhere in the source code).

HeinrichApfelmus avatar Sep 30 '22 19:09 HeinrichApfelmus

Honestly, I think this is overly conservative. We could put network ids in Event and Behavior and check that at runtime, giving a good error message. My problem with this reasoning is it's not at all complete - I can still circumvent this by using an IORef or something, so life just becomes harder without actually really solving anything. Personally I think the better documentation is to say that compile should never return Events or Behaviors.

ocharles avatar Sep 30 '22 19:09 ocharles

but eventually decided that this parameter made so much noise in the types that we would be better of with disallowing return values in compile

You could've just removed the "Frameworks t" constraint to get rid of most of the noise. The ST monad is the canonical example of using a type parameter to prevent leaks between different invocations; it doesn't use a type constraint.

ChristopherKing42 avatar Oct 01 '22 03:10 ChristopherKing42

I agree with @ocharles, but I get where @HeinrichApfelmus is coming from. I think the pattern of "don't return this value here, which we do not attempt to prevent with the type system" is common (e.g. whenever acquiring a resource that's only usable before it's released/finalized).

And @ChristopherKing42, the "ST monad trick" is indeed how the old API used to work, and is precisely what "so much noise in the types" refers to - not the one Frameworks t constraint required at the call site of compile. And I'm sure this was not your intent, but I'd like to provide you with the feedback that beginning a comment with "you could've just <alternative>" in response to a justification for why a decision was made can come across as a bit rude, especially in textual form where one cannot discern your tone of voice ;)

mitchellwrosen avatar Oct 02 '22 17:10 mitchellwrosen

Right, that makes sense

And I'm sure this was not your intent, but I'd like to provide you with the feedback that beginning a comment with "you could've just " in response to a justification for why a decision was made can come across as a bit rude

Oh, sorry about that! Yeah, I was a bit rude in retrospect, sorry.

ChristopherKing42 avatar Oct 03 '22 18:10 ChristopherKing42