Drasil icon indicating copy to clipboard operation
Drasil copied to clipboard

Handling of `State` actions in GOOL C++ renderer

Open hrzhuang opened this issue 1 year ago • 5 comments

I think this is easiest to explain by an example.

function in GOOL declares a function. It has the following type signature.

function :: Label -> r (Scope r) -> VSType r -> [MSParameter r] -> MSBody r -> SMethod r

In the C++ renderer, it is defined as follows.

function n s t = pairValListVal 
  (function n (pfst s)) (function n (psnd s)) 
  (zoom lensMStoVS t)

The gist of this is that we are pairing function for C++ source code (.cpp files) with function for C++ header code (.hpp files).

Note that the types VSType r, MSParameter r, MSBody r, and SMethod r are all type aliases for values in the State monad. The way pairValListVal handles this is that it unwraps the State (runs the State action) for each of the arguments in the order they are declared, then it's able to extract the two parts of the pair (one for the source and one for the header). After this, it wraps the values back into State (using pure) before passing them to the source functionand the header function, since all of the functions have the same type signature (r varies based on the target programming language, or in this case source vs. header).

The problem is that since pairValListVal runs the State actions before ever passing them to the source/header level function, we do not get to decide the order in which to run the State actions at the source/header level. This bothers me since at the source/header level function still takes State values as arguments, which is deceptive since at this point the original State actions have already been run and all that's left is a pure value wrapped in pure.

With respect to the work I am currently doing on name generation (see #3475 for an overview), I ran into a problem where the name generation in the body of a function are being run before I could mark the parameter names of the function as having been used. There are alternative approaches to take here, but this is just an example of strange implications of the way we currently handle State.

The problem is also not limited to pairValListVal. Most of the C++ rendering functions are written using similar functions such as pair1, pair1Val1List, etc. that join a source-level function with a header-level function, and all of these functions handle the State monad in the same way.

I am not sure this problem is fixable without significantly changing the way C++ is handled in GOOL. The source and the header share the same state so with the way things are now it's impossible for the source and the header to sequence the State actions differently. I did want to write up this issue to gather any thoughts and for future reference. I also realize that there is quite a bit of GOOL nitty-gritties here that I haven't been able to get away from, and I'm happy to clarify any points that are unclear.

hrzhuang avatar Jun 22 '23 18:06 hrzhuang

This is an excellent write-up.

First question: do you need to sequence the State actions differently for source and header? [I'm guessing yes, but I'd like a concrete example.]

Yes, this is likely to entail big changes to all these pair combinators, if not an even bigger refactor. Let's first try to see if we can begin by changing how the sequencing of state actions happen.

JacquesCarette avatar Jun 23 '23 20:06 JacquesCarette

The problem I've encountered has less to do with needing to sequence State actions differently in the source and the header, but rather that the one place where I would like to make the changes exist at the source/header level instead of at the pair level.

For example, a reasonable thing I might want to do for name generation is to mark the name of the function/method we are in as a used variable name inside the function/method. I would have to change function, method, and mainMethod separately. The shared logic between these 3 functions are in cppsIntFunc, and I would like to change this single place, but this exists at the source level. Since the State actions in the function body (i.e. generating variable names) get run at the pair level before any source/header level functions get called, it is too late to mark the name as used in cppsIntFunc.

I am sure there are workarounds we can do, but my main gripe is that it is deceptive to say "here are some States" and then "oops I've already run them." The issue I mentioned about marking function parameters as used took me a long time to finally track down to this issue of sequencing States 😅 (though I did find a better way to do it in #3502).

hrzhuang avatar Jun 23 '23 21:06 hrzhuang

It is perhaps the case that the pair combinators are just wrong, and we didn't notice because very little state was actually used, i.e. almost all the computations are in fact pure.

JacquesCarette avatar Jun 28 '23 18:06 JacquesCarette

Reading this brought back memories of debugging issues in the C++ renderer. Even when I was working with GOOL every day, it took a lot of mental effort to understand what was going on. Your experience brought back a particular memory about the states running in unexpected ways, and I thought it might be relevant, so I dug up this PR where I documented the memory: https://github.com/JacquesCarette/Drasil/pull/1965. After reading it again, though, I'm doubtful that it sheds any light on what you found.

Anyway, after I found that, I dug more in the C++ renderer code to refresh my memory of it. I definitely still don't have as much understanding as I used to, but I'll pose the question that I ended up on:

Looking at the pair1 definition, isn't the state run after the source/header functions are called?:

pair1 :: (Pair p) => (SrcState r a -> SrcState s b) -> (HdrState r a -> 
  HdrState s b) -> PairState s p a -> PairState s p b
pair1 srcf hdrf stv = do
  v <- stv                        -- state transitions at pair-level run here
  let fp = pure $ pfst v
      sp = pure $ psnd v
  p1 <- srcf fp                 -- state transitions at source level run here
  p2 <- hdrf sp                -- state transitions at header level run here
  pure $ pair p1 p2

And (I think) all of these pair helpers end up running pair1 or a similar "base" pair helper which runs the state transitions.

I'm hesitating to post this because I feel like I'm still far from where I used to be and where I'd like to be in terms of understanding this fully, and I don't want to waste anyone's time, but I figure it's worth it just in case I'm right. And now I'm curious to know why I'm wrong, if I am 😁

bmaclach avatar Jun 29 '23 04:06 bmaclach

I think this analysis is right about what pair1 does. The tricky part is what it does "in context": whatever state actions were "put in to" stv will be run first. Then the source and header actions will be run using that state.

Maybe what needs to be done is create a very small version of all of this (i.e. an abstract pair1 combinator that works over an arbitrary State monad) to create a small piece of code that can be experimented with, with respect to timing of effects.

JacquesCarette avatar Jun 29 '23 20:06 JacquesCarette