Stabilize impl From<[(K, V); N]> for HashMap (and friends)

[bstrie]

In addition to allowing HashMap to participate in Into/From conversion, this adds the long-requested ability to use constructor-like syntax for initializing a HashMap:

let map = HashMap::from([
    (1, 2),
    (3, 4),
    (5, 6)
]);

This addition is highly motivated by existing precedence, e.g. it is already possible to similarly construct a Vec from a fixed-size array:

let vec = Vec::from([1, 2, 3]);

...and it is already possible to collect a Vec of tuples into a HashMap (and vice-versa):

let vec = Vec::from([(1, 2)]);
let map: HashMap<_, _> = vec.into_iter().collect();
let vec: Vec<(_, _)> = map.into_iter().collect();

...and of course it is likewise possible to collect a fixed-size array of tuples into a HashMap (but not vice-versa just yet):

let arr = [(1, 2)];
let map: HashMap<_, _> = std::array::IntoIter::new(arr).collect();

Therefore this addition seems like a no-brainer.

As for any impl, this would be insta-stable.

[rust-highfive]

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

[bstrie]

Note that some HashMap impls are defined for all S: BuildHasher, others for only S = RandomState. The latter seemed most appropriate here (to match HashMap::new and HashMap::with_capacity), but if that seems too restrictive then let me know.

[workingjubilee]

Note that some HashMap impls are defined for all S: BuildHasher, others for only S = RandomState. The latter seemed most appropriate here (to match HashMap::new and HashMap::with_capacity), but if that seems too restrictive then let me know.

If I remember the interaction between impls and stability, it would be forward compatible with a later generalization to all S: BuildHasher, correct?

[bstrie]

You would also need to change the signature itself so that it could accept a hasher: HashMap::from(([(1, 2)], hasher)). The ergonomic hit here seems so severe that it seems hardly worth it. However, I mention it on the off-chance that somebody out there really, really likes to provide their own hasher (though I suggest that a new inherent fn like HashMap::from_with_hasher might be more appropriate if they just want the constructor syntax).

[bstrie]

Also, note that this matches the semantics of vec.into_iter().collect() in that duplicate keys in the input array are not any kind of error, and will result in the value of the last of the duplicate keys in the array being present in the map.

This also reinforces the RandomState choice, since collect doesn't worry about giving users the ability to specify a hasher.

[bstrie]

Speaking of collect, I just took a look at the implementation of FromIterator and it uses map.extend rather than iterating manually; is that preferred? It occurs to me that I could also just use collect here. The for loop seems like it might be the most straightforward to optimize, but let me know if I'm mistaken; I'm not good enough at assembly to compare the outputs of each approach.

[bstrie]

If someone wants to compare the generated assembly of alternative implementations, I whipped up what are hopefully representative examples:

• using for: https://play.rust-lang.org/?version=nightly&mode=release&edition=2018&gist=fc1078df5bcb98d71625a093840b4b6d
• using extend: https://play.rust-lang.org/?version=nightly&mode=release&edition=2018&gist=9d8362aeca57ed734894e01e5a02520c
• using collect: https://play.rust-lang.org/?version=nightly&mode=release&edition=2018&gist=7c71e285a87ad7e0656ab34a6420b502

[fbstj]

is what happens on duplicate keys the expected behaviour?

[leonardo-m]

this adds the long-requested ability to use constructor-like syntax for initializing a HashMap:

Perhaps in Rust such constructor syntax should give a compile-time error if two keys are equal inside the literal.

[bstrie]

@fbstj, the semantics here match what happens when you collect a Vec of tuples into a HashMap: elements are inserted in iteration order, so any duplicate keys will result in the value of the last duplicate key in the input.

@leonardo-m , if Rust ever had first-class HashMap literal syntax then that could be useful, but this particular API is about converting an array into a HashMap, and precedence says that duplicate keys are not considered an error. I imagine there could be a clippy lint if this syntax becomes a popular way of constructing HashMaps.

[Mark-Simulacrum]

r? @joshtriplett for T-libs FCP

I don't think determining whether we use for/extend/collect matters too much for this PR, and they all look about equivalent assembly wise (as might be expected/hoped).

[joshtriplett]

@rfcbot merge

[rfcbot]

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se
• @sfackler

Concerns:

• consistency resolved by Stabilize impl From<[(K, V); N]> for HashMap (and friends) #84111 (comment)
• documentation-fixes resolved by Stabilize impl From<[(K, V); N]> for HashMap (and friends) #84111 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[Amanieu]

For consistency, shouldn't we also implement similar From impls for BTreeMap, BTreeSet, HashSet, VecDeque, BinaryHeap, etc? Impls already exist for Vec and Box<[T]>.

@rfcbot concern consistency

[joshtriplett]

I absolutely think we should. Do you feel we need to put in all of those impls simultaneously, blocking this one on the rest?

[Amanieu]

We might as well, I don't think it makes sense to only add this impl for some collections but not others.

[sfackler]

Agree we should add impls to all of our collection types, and I think we may as well just do it all at the same time. Should be a pretty quick copy/paste job.

[sfackler]

I'd just delegate to collect here, which is where all of our specialization-based optimizations tend to live.

[bstrie]

Sure thing. Some cursory examination of the assembly revealed that collect was doing "something" a bit differently than the for or extend implementations, but I couldn't tell if it was either better or worse.

[bstrie]

For consistency, shouldn't we also implement similar From impls for BTreeMap, BTreeSet, HashSet, VecDeque, BinaryHeap, etc? Impls already exist for Vec and Box<[T]>.

Sure, I'll add that to this PR.

[cuviper]

You would also need to change the signature itself so that it could accept a hasher: HashMap::from(([(1, 2)], hasher)).

You could implicitly default the hasher, just as FromIterator uses S: BuildHasher + Default.

[cuviper]

Or if we end up adding FromIterator to the prelude, and we figure out IntoIterator for arrays (#65819 / #84147), then HashMap::from_iter([...]) will just work.

[JohnTitor]

Well, the failure is spurious but it's a good time to fix the above :)

[bstrie]

@JohnTitor Ah, we were just wondering this morning whether the 1.55 beta had been cut, we looked at https://forge.rust-lang.org/#current-release-versions and thought we were in the clear. I've pushed an update with the correct versions, can you re-approve?

[JohnTitor]

Thanks for the update!
@bors r=joshtriplett

[bors]

📌 Commit 1b83fed has been approved by joshtriplett

[bors]

⌛ Testing commit 1b83fed with merge 2b4196e...

[bors]

☀️ Test successful - checks-actions
Approved by: joshtriplett
Pushing 2b4196e to master...