Daniel James wrote:
There have been occasional issues where Boost.Hash will incorrectly hash an object that doesn't have a hash function but is convertible to bool.
That's bad and should certainly be fixed.
I added an extra templated overload of hash_value which contains a static assert. The idea is that it's a better match than the implicit cast, so it will cause a compile error in this case. This is currently only activated when a macro is defined, as it could break otherwise valid code. I was a bit worried about making any interface changes because at this stage, Boost.Hash should be very stable from release to release. The problem turned up again yesterday, so I'm starting to think it's enough of a problem that it's worth activating this be default.
I applaud your desire to avoid breaking code, but there may be a problem. If the header is included in different places with different definitions for the macro, won't that violate the ODR? All code linked into an application, whether statically or dynamically, must use the same definition for the macro. I can imagine many scenarios which would violate that precondition without warning.
A different solution might be to change the implement of Boost.Hash for bools, so that the bool overload isn't required. But that would be treating the sympton, as other implicit casts could cause problems.
You can make all of the existing overloads into function templates and use SFINAE to select the specific type in each case. That is, whereas there is now a bool overload, make it a function template that uses SFINAE to enable it only when the template parameter is bool. That approach would disallow any implicit conversions to the built-in types for which there are now overloads. That is a breaking change, of course. You could also use std::numeric_limits<T>::is_specialized to detect a possibly larger set of types to which overloads in the details namespace could be applied.
An example of valid code that will now break is something like:
struct base; std::size_t hash_value(base const&);
struct derived : base { void some_method_but_no_extra_state(); };
boost::hash<derived> derived_hash;
Currently that will automatically use base's hash_value, with this change it will be a compile error. Although that might be a good thing, as sometime it is an error to use the base's hash function.
Forcing a conscious choice is a good thing and it applies to my function template + SFINAE idea, too. Unfortunately, this does mean many currently valid uses will no longer compile. _____ Rob Stewart robert.stewart@sig.com Software Engineer using std::disclaimer; Dev Tools & Components 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.