Is specializing std::hash outside namespace std unsafe?

Question

I know that we can only add code to namespace std that is a template specialization of templates defined there otherwise the behavior is undefined.

I have implemented this code from C++ primer:

//namespace std
//{
    template <>
    class std::hash<Sales_data>
//   class hash<Sales_data>
    {
    public:
        using result_type = std::size_t;
        using argument_type = Sales_data;
        result_type operator()(argument_type const& ) const;
    };

    typename hash<Sales_data>::result_type
    hash<Sales_data>::operator()(argument_type const& arg) const
    {
        return hash<std::string>()(arg.isbn_) ^
                hash<unsigned int>()(arg.nCopySold_) ^
                hash<double>()(arg.revenue_);
    }
//}

the code works fine so am I injecting code to namespace std: template <> class std::hash<Sales_data> or simply I am specializing std::hash outside the namespace? The original code uses is with lines un-commented.
Shouldn't I do this or it is erroneous to do so?

_"...Specializations of std::hash for program-defined types must satisfy Hash requirements...."_ https://en.cppreference.com/w/cpp/language/extending_std and https://en.cppreference.com/w/cpp/utility/hash — Richard Critten, Dec 28 '20 at 23:16
@RichardCritten Are you implying that this one doesn't? I tend to agree when it comes to "_For two different parameters k1 and k2 that are not equal, the probability that `std::hash()(k1) == std::hash()(k2)` should be very small, approaching `1.0/std::numeric_limits::max()`_" - but failing to fulfill that will only cause an unnecessary amount of collisions. — Ted Lyngmo, Dec 28 '20 at 23:24

Ted Lyngmo · Accepted Answer · 2020-12-29T01:04:34.897

Is specializing std::hash outside namespace std unsafe?

No. template<> class std::hash<Sales_data> { ... }; in the global namespace will result in the same as

namespace std {
template<>
class hash<Sales_data> { ... };
}

in the global namespace.

Shouldn't I do this or it is erroneous to do so?

It's fine to do so - but older versions of g++ (versions before version 7 I think, which are still in use) will produce a warning along the lines "warning: specialization of template in different namespace" if you do it like you do in the question. Combined with -Werror this could cause perfectly correct code to fail to compile. I suggest using the namespace { ... } version.

A note on the hash itself: std::hash<any_type_smaller_or_of_equal_size_of_size_t> is likely to be a very poor hash (changing 1 bit won't flip approximately half of the others) in the standard library, returning the same output (bitpattern) as it got as input. This is common because it's extremely fast and fulfills all that's needed when used as a key in a standard container. Every value will get a unique hash, so it's just perfect from that point of view. The problem comes when combining these together. Collisions will be very easy to create - and you want to keep collisions down to a minimum to make lookups in your unordered containers fast. Your XOR operations doesn't change the fact about collisions being plentiful, so I suggest using a function to combine hashes. boost has one that is commonly used (boost::hash_combine).

It looks something like this for 32 bit size_ts (not sure if it's different in different boost versions):

template <class T>
inline void hash_combine(std::size_t& seed, const T& v)
{
    std::hash<T> hasher;
    seed ^= hasher(v) + 0x9e3779b9 + (seed << 6) + (seed >> 2);
}

Example:

namespace stolen { // from boost - use the real boost version to get proper 64 bit hashes
    template <class T>
    inline void hash_combine(std::size_t& seed, const T& v)
    {
        std::hash<T> hasher;
        seed ^= hasher(v) + 0x9e3779b9 + (seed << 6) + (seed >> 2);
    }
}

namespace std {
    template <>
    class hash<Sales_data> {
    public:
        using result_type = std::size_t;
        using argument_type = Sales_data;

        result_type operator()(argument_type const& arg) const {
            result_type result = hash<std::string>{}(arg.isbn_);
            stolen::hash_combine(result, arg.nCopySold_);
            stolen::hash_combine(result, arg.revenue_);
            return result;
        }
    };
}

There's also a proposal to add some version of hash_combine into the standard library: hash_combine() Again

One last thing: If I just want in a header file to declare that specialization should I do: `namespace std{template <> class hash;}` or `namespace std{template class hash;}` then in a source I define that specialization like: `namespace std{template <> class hash{};// definition}`. I think the second one of declaration is just re-declaring the primary template which is redundant although correct. What do you think? Thank you. — Itachi Uchiwa, Dec 29 '20 at 12:02
@ItachiUchiwa If you move the implementation to a `.cpp` file, you must explicitly instantiate it for the types you aim to support. This is rarely what one wants. More on that: [Why can templates only be implemented in the header file?](https://stackoverflow.com/questions/495021/why-can-templates-only-be-implemented-in-the-header-file) - but perhaps it'd be ok in this case since you know exactly what to instantiate it for. Check the [Class template - Explicit instantiation](https://en.cppreference.com/w/cpp/language/class_template) section. — Ted Lyngmo, Dec 29 '20 at 18:35
Ok now. The book said: "The only complication in defining this hash specialization is that when we specialize a template, we must do so in the same namespace in which the original template is defined." So it is wrong. right? — Itachi Uchiwa, Dec 29 '20 at 20:31
But the book said " we must do so in the same namespace in which the original template is defined."? — Itachi Uchiwa, Dec 29 '20 at 20:51
@ItachiUchiwa Yes, and that's exactly what you did when you did `namespace std { template<> class hash { ... }; }` - so the book is correct and you did it right. — Ted Lyngmo, Dec 29 '20 at 20:54
But what's about this: `template<> class std::hash { ... };` I think this correct too because I've seen an example from cppreference.com: https://en.cppreference.com/w/cpp/language/extending_std and am I here defining the specialization inside the namespace std? I have an ambiguity about the latter version: `template<> class std::hash;`. — Itachi Uchiwa, Dec 29 '20 at 20:57
Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/226579/discussion-between-ted-lyngmo-and-itachi-uchiwa). — Ted Lyngmo, Dec 29 '20 at 21:01
Ok get it now! the latter doesn't work on versions of G++ below 7.1 with pedantic flags. Thanks a lot! — Itachi Uchiwa, Dec 29 '20 at 21:22

Is specializing std::hash outside namespace std unsafe?

1 Answers1