Peter Dimov wrote:
First of all, nice work!
Thanks. I was planning on asking for a post-review once I'd finished the documentation. I rushed the headers and tests into CVS early so that MultiIndex can use them, and I can get regression test results - otherwise I won't make the 1.33 release schedule. I mostly agree, or feel indifferent, about everything you wrote. So only a couple of quick responses.
* hash/hash.hpp contains hash_range overloads for Range objects
I don't recall this being discussed during the review.
It was, but very briefly. (At least, it somehow made its way into my notes).
This introduces a dependency on the range headers. Given that std::pair is separated in its own header, this is a bit odd, especially when the range headers include <utility>.
But what's more important, these overloads are unnecessary. The proper protocol to obtain the hash value of an object x is hash_value( x ), not hash_range( x ). Consider the case std::vector<X>. If X does not support hash_value, hashing the vector will fail, even in the case where hash_range could have been used for X.
If a range object is not supported by hash_value, we just need to add a hash_value overload for it.
Summary: these overloads need to be removed.
Well, I'll do this, unless anyone comes up with a good reason not to.
* boost::hash derived from std::unary_function
This introduces a completely unnecessary dependency on <functional>. boost::hash only needs to define result_type and argument_type.
I thought this was required by TR1. I'll happily remove it if isn't. Daniel