Implement get_disjoint_mut (previously get_many_mut)

[NiklasJonsson]

std::collections::HashMap is adding get_many_mut in rust-lang/rust#97601 and I'm working on replacing some uses of that hash map with this one where some of the code uses get_many_mut (currently only a single location in rustc) so I would appreciate it if indexmap::IndexMap could provide the same interface.

This code uses a decent amount of unsafe compared to the rest of the file and I saw the comment in lib.rs on having almost all unsafe code in raw.rs but I couldn't figure out how to move parts of my implementation there as it is mostly dealing with initializing the array. I tried to use array::map to avoid all unsafe but couldn't quite get it to work with the lifetimes and the lambda.

If you want this change, I'll also write some docs.

[cuviper]

I think ideally we would want two things for this, especially the first:

1. Stabilizing HashMap::get_many_mut so we can be sure to match that API as much as possible.
2. Stabilizing <[T]>::get_many_mut (still new in Add slice methods for indexing via an array of indices. rust-lang/rust#83608) to deal with all the unsafe aspects. We might also want to wrap that into our own get_many_index_mut, just like we have get_index and get_index_mut for slice-like indexing.

I tried to use array::map to avoid all unsafe but couldn't quite get it to work with the lifetimes and the lambda.

I suspect part of this is because you should be using a raw pointer to offset/index each of the elements, otherwise you have the main &mut [T] aliasing each of the &mut T you pull out while you're working. And of course, using raw pointers will still need unsafe to form the final references.

[NiklasJonsson]

Stabilizing HashMap::get_many_mut so we can be sure to match that API as much as possible.

Yeah, this makes a lot of sense. Is it okay to just leave the PR open until then?

Stabilizing <[T]>::get_many_mut

Haven't seen this before but it would indeed fit pretty well. It seems to have been open for quite a while without resolution though. Also, if they opt for a result-based return value, that'd not match the hashmap api (as it is now) so we'd have to do conversions in the implementation.

I suspect part of this is because you should be using a raw pointer to offset/index each of the elements, otherwise you have the main &mut [T] aliasing each of the &mut T you pull out while you're working. And of course, using raw pointers will still need unsafe to form the final references.

Yeah, exactly, the lambda captures the whole array lifetime mutably and can't be invoked multiple times. I did not consider pointer arithmetic though, I'll experiment a bit and see how it looks. Probably similar to the <[T]>::get_many_mut examples 🙂

[cuviper]

Is it okay to just leave the PR open until then?

Yeah, that's fine. I should probably make a label for this...

[jyn514]

I think ideally we would want two things for this, especially the first: Stabilizing HashMap::get_many_mut so we can be sure to match that API as much as possible.

Can IndexMap add it under a nightly or unstable opt-in cargo feature? Then people will have to opt in to breaking changes, and you can document that changes to this API won't result in a new major version.

(Or if you don't feel strongly, you can just make a new major release if the HashMap API changes for some reason.)

[cuviper]

I don't like using nightly/unstable features, in part because it's an extra maintenance burden, but also because the feature may be opted-in on your behalf, arbitrarily deep in your dependency tree.

I'd rather not set us up for needing a semver bump either. If this is something that folks really need, let's apply that pressure on std stabilization. :)

[cuviper]

Rust 1.86 (now in beta) will stabilize this as get_disjoint_mut with two different styles:

For [T]: https://doc.rust-lang.org/beta/std/primitive.slice.html#method.get_disjoint_mut

pub fn get_disjoint_mut<I, const N: usize>(
    &mut self,
    indices: [I; N],
) -> Result<[&mut <I as SliceIndex<[T]>>::Output; N], GetDisjointMutError>
where
    I: GetDisjointMutIndex + SliceIndex<[T]>,

For HashMap<K, V>: https://doc.rust-lang.org/beta/std/collections/struct.HashMap.html#method.get_disjoint_mut

pub fn get_disjoint_mut<Q, const N: usize>(
    &mut self,
    ks: [&Q; N],
) -> [Option<&mut V>; N]
where
    K: Borrow<Q>,
    Q: Hash + Eq + ?Sized,

We can decide for ourselves whether it makes sense to build on the standard library, or (unsafely) implement our own. GetDisjointMutIndex won't be stable though, nor the old SliceIndex, so if we want to replicate that support for both usize and ranges, then I think we'll be on our own anyway.

@NiklasJonsson it's been some time -- are you still interested in working on this?

[cuviper]

We can decide for ourselves whether it makes sense to build on the standard library, or (unsafely) implement our own. GetDisjointMutIndex won't be stable though, nor the old SliceIndex, so if we want to replicate that support for both usize and ranges, then I think we'll be on our own anyway.

Either way, I'd be fine with supporting just usize to start with, as long as there's a possibility to add ranges later. But even that could be separate, like how we have get_index_mut and get_ranges_mut covering the same as slice get_mut (and our get_mut matches HashMap). In other words, we could leave that extension to a future get_disjoint_ranges_mut.

[NiklasJonsson]

Yeah I've been following the stabilization and I'd like to finish this up. I'll have some time this weekend to have a deeper look but I think I'll favor supporting less to start out and then I'll see if I have time to do more.

…

On Tue, Feb 25, 2025, 02:29 Josh Stone ***@***.***> wrote: Rust 1.86 (now in beta) will stabilize this as get_disjoint_mut with two different styles: For [T]: https://doc.rust-lang.org/beta/std/primitive.slice.html#method.get_disjoint_mut pub fn get_disjoint_mut<I, const N: usize>( &mut self, indices: [I; N],) -> Result<[&mut <I as SliceIndex<[T]>>::Output; N], GetDisjointMutError>where I: GetDisjointMutIndex + SliceIndex<[T]>, For HashMap<K, V>: https://doc.rust-lang.org/beta/std/collections/struct.HashMap.html#method.get_disjoint_mut pub fn get_disjoint_mut<Q, const N: usize>( &mut self, ks: [&Q; N],) -> [Option<&mut V>; N]where K: Borrow<Q>, Q: Hash + Eq + ?Sized, We can decide for ourselves whether it makes sense to build on the standard library, or (unsafely) implement our own. GetDisjointMutIndex won't be stable though, nor the old SliceIndex, so if we want to replicate that support for both usize and ranges, then I think we'll be on our own anyway. @NiklasJonsson <https://github.com/NiklasJonsson> it's been some time -- are you still interested in working on this? — Reply to this email directly, view it on GitHub <#238 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADNRXP6VM3HKI4SGOGBRNOD2RPBONAVCNFSM6AAAAABXZNCFMWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMOBQGEZDQMRTGI> . You are receiving this because you were mentioned.Message ID: ***@***.***> [image: cuviper]*cuviper* left a comment (indexmap-rs/indexmap#238) <#238 (comment)> Rust 1.86 (now in beta) will stabilize this as get_disjoint_mut with two different styles: For [T]: https://doc.rust-lang.org/beta/std/primitive.slice.html#method.get_disjoint_mut pub fn get_disjoint_mut<I, const N: usize>( &mut self, indices: [I; N],) -> Result<[&mut <I as SliceIndex<[T]>>::Output; N], GetDisjointMutError>where I: GetDisjointMutIndex + SliceIndex<[T]>, For HashMap<K, V>: https://doc.rust-lang.org/beta/std/collections/struct.HashMap.html#method.get_disjoint_mut pub fn get_disjoint_mut<Q, const N: usize>( &mut self, ks: [&Q; N],) -> [Option<&mut V>; N]where K: Borrow<Q>, Q: Hash + Eq + ?Sized, We can decide for ourselves whether it makes sense to build on the standard library, or (unsafely) implement our own. GetDisjointMutIndex won't be stable though, nor the old SliceIndex, so if we want to replicate that support for both usize and ranges, then I think we'll be on our own anyway. @NiklasJonsson <https://github.com/NiklasJonsson> it's been some time -- are you still interested in working on this? — Reply to this email directly, view it on GitHub <#238 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADNRXP6VM3HKI4SGOGBRNOD2RPBONAVCNFSM6AAAAABXZNCFMWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMOBQGEZDQMRTGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[cuviper]

Sounds great, thanks! And please don't take that as any pressure to do this quickly -- we can take our time and get it right.

[cuviper]

2. We might also want to wrap that into our own get_many_index_mut,

Correcting myself -- this should be plural, so now it would be get_disjoint_indices_mut.

[NiklasJonsson]

@cuviper Just to verify that I understand what you mean. An alright initial implementation would be:

1. The key-based get_disjoint_mut:

   pub fn get_disjoint_mut<Q, const N: usize>(
       &mut self,
       ks: [&Q; N],
   ) -> [Option<&mut V>; N]
   where
       K: Borrow<Q>,
       Q: Hash + Eq + ?Sized,

2. A get_disjoint_indices_mut that takes a usize array (pretty much the current get_many_index_mut)

and then we can leave the implementation of get_disjoint_mut with ranges/for a future PR? If so, then it seems to be mostly about renaming the functions.

[NiklasJonsson]

Just to verify that I understand what you mean. An alright initial implementation would be:
...

I went ahead and implemented what I understood you to mean 🙂 Also added some extra tests.

[cuviper]

Ok, Rust 1.86.0 shipped for us to match API, and I pushed the last few review suggestions myself. I think this is ready to go -- thank you for your contribution!

[NiklasJonsson]

Ok, Rust 1.86.0 shipped for us to match API, and I pushed the last few review suggestions myself. I think this is ready to go -- thank you for your contribution!

Thanks, happy to help!