-
-
Notifications
You must be signed in to change notification settings - Fork 419
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
Make Map insertion functions total #3203
Make Map insertion functions total #3203
Conversation
f2bd17b
to
a87a371
Compare
@rkallos you'll need to rebase against master to pick up the removal of pcre2 so that when you start up on this again, it can work with the latest CI images. |
d562073
to
098dec1
Compare
Thanks @SeanTAllen for adding the label and letting me know to rebase. I think this is ready for review now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a beautiful thing!
811b3b8
to
acea091
Compare
Rebased and squashed :) |
@rkallos can you add release notes for this change? including "what your code looks like before and after" so folks know how to update? |
Before this change, code that inserts into maps might look like:
After this change, the question marks and the surrounding
|
Thanks to #3201, it's now possible to make Map.insert & friends total functions. Not only that, but Map.insert should be slightly faster in the case where the insertion causes the underlying array to grow, as it is no longer necessary to do the lookup for the final value.
I suspect this would constitute a pretty large breaking change, as I'm sure code like
try map.insert(k, v)? end
is pretty common, and after this change it would be a compile-time error.This PR is incomplete. There are some more partial-not-partial functions that can be fixed, like Map.insert_if_absent.