-
Notifications
You must be signed in to change notification settings - Fork 32
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
Key constraint might not be ideal #25
Comments
List has to be dropped because type aliases are not allowed for generic types. There's an outstanding issue that Set can't contain pointer values. benbjohnson/immutable#25 I would abandon this package altogether, but there's no Set type in immutable, and its comparer and hasher types are more boilerplate than I want.
Hi @anacrolix . The problem was that SortedMap requires Maybe it's not the best approach long-term, but Ben seemed happy with this for now.. Unless there's something I'm missing - maybe I am - I think you'd need to duplicate all the map-related types like Hasher, defaultHasher, MapIterator, mapEntry, mapNode, mapValueNode, mapLeafNode, mapArrayNode, MapBuilder, ... there's a lot of types in there. Hopefully there's some opportunities to avoid some of the duplication, but it's an undertaking - feel free to take it on. At least you know why it is like it is now :) |
I don't have a strong opinion on any of this—mostly because I haven't done much with generics. If anybody has a good solution then I'm open to it but I haven't had time to commit to the |
I definitely don't have the time to dedicate to this right now. It's used in the old and obsoleted STM concurrency implementation in anacrolix/dht that I think only gets dredged up because of golang/go#55955. |
@anacrolix I had a quick experiment with widening the constraint for immutable.Map. Does it help? |
I will give it a go and report back, thank you @laher ! |
I gave it a spin, it didn't work for the underlying user (although the stm code passed). I think the type can be widened for SortedMap (and any other variants too). Again, because comparers are manually given, the restriction on types belong to those implementations. I'm pretty sure it works for the builtin comparers too, as those use type sets to check for orderable types anyway. Short answer: I think |
Good shout - I think it should be OK, just needing a bit of jiggery-pokery on the defaultComparer. |
Let me know if you do that change, and I'll report back. Thanks @laher |
I have a wrapper of immutable that adds sets and some other generic interface stuff. I'm having difficulty mapping it onto v0.4.0 of immutable. I read the PR for the generics, and I think that constraints.Ordered isn't quite right for the key values. I haven't looked into it more, but I suspect it's too strict and that
comparable
should be possible instead, particularly because the user already has to provide comparer/hasher types in the constructors.@laher
The text was updated successfully, but these errors were encountered: