-
Notifications
You must be signed in to change notification settings - Fork 25
Remove STRef
, unify with Ref
#30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I’d really prefer to not make this such a large breaking change if we possibly can. Can we consider continuing to have two separate Ref types, one in ST and one in Refs, which are the same underneath - perhaps one is a newtype of the other? That way you still get to share the implementation and also allow converting between them if that’s something you care about, but you also get to continue not to care that they’re the same underneath if that’s irrelevant to your use case (which, in my experience, it is most of the time). |
I get not wanting to break things unnecessarily, but also I don't think we should maintain situations that are less than ideal just to avoid breaking changes, especially if we're making a batch of breaking changes anyway. Another step in the above that is something I know you've been a proponent of in the past: renaming the modules to match the library names rather than having |
Do we need optimizer rules for MonadST? |
Do we even have optimizer rules for ST at the moment? They got lost when we changed from |
Ah ok, great. In that case, if there is some kind of sensible optimization for |
I think that we would want to apply optimizations based on which dictionary is chosen (whether for Effect or ST) and inline appropriately. |
Ideally there wouldn't be a penalty for using MonadST even in a monomorphic setting. |
A proposal that @natefaubion suggested and I had also mulled but disregarded: to make the change less breaking we could exports synonyms and such from the existing name modules, as existing code should still work then. |
Now there's the
Global
region,MonadST
, etc. we can do away with having twoRef
types:Ref
inpurescript-refs
to includeRegion
(basically,STRef
->Ref
)Ref.Lifted
module where the operations have aMonadST
constraint so they can be used inEffect
without friction.STRef
frompurescript-st
MonadRec
instance currently inpurescript-st
, as it currently is implemented withSTRef
. Implement it inpurescript-tailrec
instead (implementation is basically identical to theEffect
instance already in there).I've done all this locally to check it works out 🙂.
I'd like to make this change when we do the updates for PS 0.14 unless there is a strong feeling we shouldn't for some reason. Aside from some module stuff moving around the main breaking difference is that existing
Ref
s will need to becomeRef Global
.The text was updated successfully, but these errors were encountered: