hashbrown icon indicating copy to clipboard operation
hashbrown copied to clipboard

Violating the set invariant with `HashSet::get_or_insert_with`

Open JustForFun88 opened this issue 2 years ago • 2 comments

The implementation of HashSet::get_or_insert_with is not good. This method make it quite easy to break the invariant of set that all elements are repeated in set only once. Let's say this test passes:

#[test]
fn some_invalid_insert() {
    let mut set = HashSet::new();
    set.insert(1);
    set.get_or_insert_with(&2, |_| 1);
    set.get_or_insert_with(&3, |_| 1);
    assert_eq!(vec![&1, &1, &1], set.iter().collect::<Vec<_>>());
}

May be we need document it?

JustForFun88 avatar Feb 15 '23 11:02 JustForFun88

There is a much longer discussion about this in https://github.com/rust-lang/rust/issues/60896, which is about the equivalent (unstable) method in the standard library. I would prefer to keep the API as it is for now until a decision is made on that issue. One proposed alternative is to drop this function and instead only provide get_or_insert_owned.

Amanieu avatar Feb 22 '23 14:02 Amanieu

There is a much longer discussion about this in rust-lang/rust#60896, which is about the equivalent (unstable) method in the standard library. I would prefer to keep the API as it is for now until a decision is made on that issue. One proposed alternative is to drop this function and instead only provide get_or_insert_owned.

Didn't know about the discussion. In any case, I'll leave #400 open for now, pending a final decision.

JustForFun88 avatar Feb 23 '23 09:02 JustForFun88