Replace IORef by MutVar
#334
Conversation
As a result, quite a few types are now parameterised over a state parameter `s` (e.g., `RealWorld` for `IO`). This change causes quite a bit of noise, but it's necessary for generalising all our code away from just `IO`. I have also considered whether it would be better to parameterise these types by a monad `m` instead of the state type `s`. An `m` parameter is slightly more general, since we can use `PrimState m` whenever `s` would be required. In the spirit of the YAGNI principle, I decided not to do that (for now). We can make that change later if turns out to be necessary.
jorisdral
force-pushed
the
jdral/mutvar-for-ioref
branch
from
07d83f2 to
3a93441
Compare
|
BTW, I'm running the micro-benchmarks locally to see if the changes lead to a regression |
Okay, as far as I can see there is no noticeable regression. |
mheinzel
left a comment
mheinzel
left a comment
There was a problem hiding this comment.
The type parameter spreads into quite a lot of types, but we will need this for io-sim, right?
| #ifdef NO_IGNORE_ASSERTS | ||
| void $ stToIO $ _levelsInvariant conf ls' | ||
| #endif | ||
| return ls' |
There was a problem hiding this comment.
Interesting, what is the advantage of using CPP here?
Is it because the invariant is not really a predicate (always returning True), so it would be weird to use assert? In that case, we could also change it to return () now and get rid of the void.
There was a problem hiding this comment.
The current levelsInvariant doesn't actually read mutable variables, so we can run it purely, but once we add ongoing merges we'll have to read mutable variables to inspect the state of an ongoing merge. As such, we can't use runST anymore, and we have to instead embed the invariant check into IO
There was a problem hiding this comment.
And reading the variable won't be possible in ST, like with MutVar? Okay, we'll see what it looks like then.
There was a problem hiding this comment.
The problem is that runST is polymorphic over s, while in this addRunToLevels function we have s ~ RealWorld (or more precisely, s ~ PrimState m), so we have to embed using stToIO
Yes, we'll need either a state parameter |
No description provided.