[functional] [shared_ptr] boost::hash<shared_ptr<T>> Returns Only Two Values
The following is a program that demonstrates that a null shared_ptr<int> produces a hash value of zero and that (ostensibly) all non-null instances produce a hash value of one. I expected that there'd be a shared_ptr specialization of hash_value to make it work right, but I don't find one. Shouldn't there be one? #include <iostream> #include <map> #include <vector> #include <boost/shared_ptr.hpp> #include <boost/functional/hash.hpp> int main() { typedef boost::shared_ptr<int> ptr_type; typedef std::map<std::size_t, int> hashes_type; std::vector<ptr_type> v(25); hashes_type hashes; boost::hash<ptr_type> hasher; ptr_type null_ptr; ++hashes[hasher(null_ptr)]; for (int i(0); i < 25; ++i) { v[i] = ptr_type(new int(i)); std::size_t const hash(hasher(v[i])); ++hashes[hash]; } for (hashes_type::iterator it(hashes.begin()), end(hashes.end()); it != end; ++it) { std::cout << it->first << ": " << it->second << '\n'; } } _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 18 February 2011 19:40, Stewart, Robert <Robert.Stewart@sig.com> wrote:
The following is a program that demonstrates that a null shared_ptr<int> produces a hash value of zero and that (ostensibly) all non-null instances produce a hash value of one.
This is known, it happens because shared_ptr can be implicitly cast to bool, so it uses that. If you set BOOST_HASH_NO_IMPLICIT_CASTS it won't happen. This macro isn't set by default because I'm not sure if there will be any other consequences from preventing implicit casts, but it might be in a future version.
I expected that there'd be a shared_ptr specialization of hash_value to make it work right, but I don't find one. Shouldn't there be one?
That's an issue for the shared_ptr implementation. Daniel
Daniel James wrote:
On 18 February 2011 19:40, Stewart, Robert <Robert.Stewart@sig.com> wrote:
The following is a program that demonstrates that a null shared_ptr<int> produces a hash value of zero and that (ostensibly) all non-null instances produce a hash value of one.
This is known, it happens because shared_ptr can be implicitly cast to bool, so it uses that. If you set BOOST_HASH_NO_IMPLICIT_CASTS it won't happen. This macro isn't set by default because I'm not sure if there will be any other consequences from preventing implicit casts, but it might be in a future version.
Thanks for the information. We knew it was due to the Safe Bool Idiom, but it was surprising that there was no documentation on the issue or a suitable solution available. Defining BOOST_HASH_NO_IMPLICIT_CASTS produces a static assertion failure which is certainly better than silent misbehavior. It would be nice to ascertain whether allowing implicit casts is really worthwhile.
I expected that there'd be a shared_ptr specialization of hash_value to make it work right, but I don't find one. Shouldn't there be one?
That's an issue for the shared_ptr implementation.
The following worked even without defining BOOST_HASH_NO_IMPLICIT_CASTS: namespace boost { template <class T> std::size_t hash_value(boost::shared_ptr<T> const & _ptr) { return boost::hash_value(_ptr.get()); } } Peter, should this be added to boost/smart_ptr/shared_ptr_hash_value.hpp or some such? _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
Daniel James wrote:
On 18 February 2011 19:40, Stewart, Robert <Robert.Stewart@sig.com> wrote:
The following is a program that demonstrates that a null shared_ptr<int> produces a hash value of zero and that (ostensibly) all non-null instances produce a hash value of one.
This is known, it happens because shared_ptr can be implicitly cast to bool, so it uses that. If you set BOOST_HASH_NO_IMPLICIT_CASTS it won't happen.
But why is there a definition of hash_value( bool ) when this macro isn't defined? Shouldn't the largest integral types be enough? Even if they aren't, everything below int shouldn't be necessary - it's a standard promotion and it shouldn't be ambiguous to omit bool/char/short. (I'll still fix this on shared_ptr's side, just wondering.)
On 24 February 2011 22:25, Peter Dimov <pdimov@pdimov.com> wrote:
But why is there a definition of hash_value( bool ) when this macro isn't defined? Shouldn't the largest integral types be enough? Even if they aren't, everything below int shouldn't be necessary - it's a standard promotion and it shouldn't be ambiguous to omit bool/char/short.
It looks like it was required for Borland.
From http://article.gmane.org/gmane.comp.lib.boost.devel/119513/
2. It is said in hash.cpp that BCB has problems with the overload of hash_value for bool. This should be fixed in one way or another (possibly by not defining it for this compiler, I guess.)
It actually says that BCB has problems without it. There was an overload ambiguity error for boost::hash<bool> (although, calling boost::hash<bool> would be a very odd thing to do) and that was added to work around it.
Daniel
Daniel James wrote:
On 24 February 2011 22:25, Peter Dimov <pdimov@pdimov.com> wrote:
But why is there a definition of hash_value( bool ) when this macro isn't defined? Shouldn't the largest integral types be enough? Even if they aren't, everything below int shouldn't be necessary - it's a standard promotion and it shouldn't be ambiguous to omit bool/char/short.
It looks like it was required for Borland.
From http://article.gmane.org/gmane.comp.lib.boost.devel/119513/
This has led me to question the hash_value design altogether (even though I am the one who came up with it). Relying solely on boost::hash specializations and not on hash_value overloads has the side effect of not allowing conversions, and as this case demonstrates, an unwanted conversion in this context can cause substantial harm, because the code still compiles and runs, causing a 100% collision rate. It seems better to just require an exact specialization.
Stewart, Robert wrote:
The following is a program that demonstrates that a null shared_ptr<int> produces a hash value of zero and that (ostensibly) all non-null instances produce a hash value of one.
You might want to file a bug on that one. :-)
Peter Dimov wrote:
Stewart, Robert wrote:
The following is a program that demonstrates that a null shared_ptr<int> produces a hash value of zero and that (ostensibly) all non-null instances produce a hash value of one.
You might want to file a bug on that one. :-)
Done: <https://svn.boost.org/trac/boost/ticket/5216> ___ Rob IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
participants (3)
-
Daniel James -
Peter Dimov -
Stewart, Robert