[libcxx-commits] [PATCH] D61771: Comparing Unordered Containers (P0809)

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/containers/unord/unord.set/comp_different_hash.pass.cpp:64
+template<class T>
+size_t hash_even     (T* val)     { return *val & 1 ? 1 : 0; }
+
----------------
FWIW, I'd rather see something simpler here. As simple as providing `hash_mod_two { return val % 2; }` and `hash_mod_three { return val % 3; }`. I don't think it buys anything to have 5 different hashes as opposed to 2, nor to have partial specializations for `T*` of which the only one that's used is `T=char`.
I don't even see any reason to test `char*`, so I'd remove all that; but if there's any value in keeping it, then there should be a reason for `c_vals[2]` to be `"cde"` and not just `"c"`.

In fact I think the whole `void test()` could be just [WARNING: UNTESTED CODE]

    using Hash = int(*)(T);
    std::unordered_set<int, Hash> c1({1, 2, 3, 4, 6, 7, 8}, 0, +[](int x) { return x % 2; });
    std::unordered_set<int, Hash> c2({1, 2, 3, 4, 6, 7, 8}, 0, +[](int x) { return x % 3; });
    std::unordered_set<int, Hash> c3({1, 2, 3, 5, 6, 7, 8}, 0, +[](int x) { return x % 3; });
    assert(!std::equal(c1.begin(), c1.end(), c2.begin(), c2.end()));
    assert(std::is_permutation(c1.begin(), c1.end(), c2.begin(), c2.end()));
    assert(c1 == c2);
    assert(!(c1 != c2));
    assert(!std::is_permutation(c1.begin(), c1.end(), c3.begin(), c3.end()));
    assert(c1 != c3);
    assert(!(c1 == c3));

followed by a test for non-default equivalence predicate:

    struct EqualMod6 { bool operator(int x, int y) const { return x % 6 == y % 6; } };
    using Hash = int(*)(T);
    std::unordered_set<int, Hash, EqualMod6> c1({1, 2, 3, 4, 6}, 0, +[](int x) { return x % 2; });
    std::unordered_set<int, Hash, EqualMod6> c2({1, 8, 9, 4, 6}, 0, +[](int x) { return x % 3; });
    assert(!std::equal(c1.begin(), c1.end(), c2.begin(), c2.end()));
    assert(!std::is_permutation(c1.begin(), c1.end(), c2.begin(), c2.end()));
    assert(!std::equal(c1.begin(), c1.end(), c2.begin(), c2.end(), EqualMod6()));
    assert(std::is_permutation(c1.begin(), c1.end(), c2.begin(), c2.end(), EqualMod6()));
    assert(c1 == c2);
    assert(!(c1 != c2));

( https://en.cppreference.com/w/cpp/container/unordered_set/operator_cmp is pretty ugly but I think it makes sense what it's saying.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61771/new/

https://reviews.llvm.org/D61771