rodrimati1992 / abi_stable_crates

Rust-to-Rust ffi,ffi-safe equivalents of std types,and creating libraries loaded at startup.
Apache License 2.0
536 stars 30 forks source link

No documentation for the `MapKey` and `MapQuery` types #84

Open marioortizmanero opened 2 years ago

marioortizmanero commented 2 years ago

I have been banging my head against the wall trying to make #83 work, and I have finally concluded that MapKey/MapQuery are at fault. I have been trying to understand it and I will eventually do, but it would have been a much nicer experience if they were documented in the first place. Currently, the only comment is this, for MapQuery, but I personally find it a bit lacking if you aren't familiar with the code:

/// A trait object used in method that access map entries without replacing them.

After reading it, I still have a few more questions after reading it: why the NotCopyNotClone marker? Why both MapKey and MapQuery? Why is this needed? Why is as_mapkey unsafe if we know that self is never going to be null? Shouldn't the MapQuery be Pinned?

The alternative is to remove them altogether, since they just seem to be a hack, though not sure if that would be possible. Any ideas regarding that? I wouldn't like to have someone go through this again.

It would also be nice to have some tests, like this one:


#[test]
fn map_key() {
    let test_key = "abc";
    let builder = DefaultHashBuilder::new();

    let query = MapQuery::<'_, &'static str>::new(&test_key);
    let mut hasher = builder.build_hasher();
    query.hash(&mut hasher);
    let query_hash1 = hasher.finish();

    let map_query = unsafe { &query.as_mapkey() };
    let mut hasher = builder.build_hasher();
    map_query.hash(&mut hasher);
    let query_hash2 = hasher.finish();

    assert_eq!(query_hash1, query_hash2);

    let map_value = MapKey::Value(test_key);
    let mut hasher = builder.build_hasher();
    map_value.hash(&mut hasher);
    let value_hash = hasher.finish();

    assert_eq!(value_hash, query_hash2);
}