absl/hash banned in DEPS?

[Joe Mason]

I noticed that third_party/abseil-cpp/absl/hash is in the banned directly list in the root DEPS file, in the section that starts with:

  # Abseil is allowed by default, but some features are banned. See

  # //styleguide/c++/c++-features.md.

But hashing isn't mentioned at all in c++features.md. Is this an oversight? 

The context is I needed to add `AbslHashValue` friend functions to some classes so they can be used as keys in absl::flat_hash_map. That itself doesn't need to include third_party/abseil-cpp/absl/hash/hash.h, but I tried to include it in a unit test  to use `absl::HashOf` to make sure the friend functions built correctly.

I could just stick objects in an absl::flat_hash_map for the unit test if there's an actual reason to ban direct use of absl/hash, but I suspect it's just banned in DEPS by mistake.

Joe

[John Admanski]

When I was making the changes to allow the abseil hash containers I don't think it even occurred to me to remove the hash library from the DEPS banlist. Probably because you don't normally need to depend on it much, not even to define your own hashes, so it simply didn't come up.

So I don't think this is a deliberate decision, just an accidental oversight? If you sent out a change to update all the various DEPS then it would probably get approved, although I'm not an owner so I can't definitively say so. I can't really think of a great reason to allow the use of the hash tables but ban (direct) use of the hashing module.

[Daniel Cheng]

Allowing it seems fine; my only real concern is if someone tries to use it as a stable hash. Hopefully tests would catch that sort of issue though.

Daniel

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To view this discussion visit https://groups.google.com/a/chromium.org/d/msgid/cxx/3da27c1a-cb57-4b0a-838a-3b40896662e5n%40chromium.org.

[John Admanski]

There's probably a interesting larger question one could ask re: should people be encouraged to use abseil hashing vs std::hash but answering that doesn't seem like a worthwhile prerequisite to allowing the use of absl/hash/hash.h.

[Joe Mason]

I ended up refactoring my tests to use absl::flat_hash_map anyway, just because they were simpler that way, so I won't poke any DEPS files now.