My std::hash for std::tuples... Any improvements?

Question

Some may have noticed that std::hash does not support tuples. So I added an overload which just seems "nicer" than the solution I saw up till now. Anyone got ideas to further cut down this code? Please note that this is a compiler killer! The only one that could compile it was "Clang 3.2"... Intel Compiler 13.1 does not get the specialization and keeps telling "C++ standard does not support hash blabla". And we don't need to talk about the original Microsoft compiler do we.

BTW, my solution supports recursive tuples like std::tuple<std::tuple<int,int>,int> so I am not sure if this also applies to the existing solutions I saw this day.

namespace std
{
    template<typename... TTypes>
    class hash<std::tuple<TTypes...>>
    {
    private:
        typedef std::tuple<TTypes...> Tuple;

        template<int N>
        size_t operator()(Tuple value) const { return 0; }

        template<int N, typename THead, typename... TTail>
        size_t operator()(Tuple value) const
        {
            constexpr int Index = N - sizeof...(TTail) - 1;
            return hash<THead>()(std::get<Index>(value)) ^ operator()<N, TTail...>(value);
        }

    public:
        size_t operator()(Tuple value) const
        {
            return operator()<sizeof...(TTypes), TTypes...>(value);
        }
    };
}

There's already a "proper" implementation in Boost's tuple. I'd just copy that. — Kerrek SB, Feb 27 '13 at 04:35
Technically, I think you are actually declaring a new template with parameters template rather than specializing the existing template with parameters template...which wasn't technically legal to do for standard templates in C++03 as far as I understand...so it's possible this is non-conforming if you leave it in the std namespace...I'm not sure if it's still the case in C++11 though — Stephen Lin, Feb 27 '13 at 04:58
See the caveats section here: http://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Non-throwing_swap (about std::swap, but should apply to std::hash too) — Stephen Lin, Feb 27 '13 at 04:59
@Kerrek: Boost unfortunately has the habit of making things a lot more complicated than they really are because the have this "old" compiler baggage to carry around. I want this to be as short and sweet as possible. — thesaint, Feb 27 '13 at 05:05
@StephenLin: Didn't really read it but I glanced at it. Since GCC 4.7 and Clang 3.2 seem to compile this, I guess it should be fine as it is, since especially Clang is known to be extremely conforming to the standard, it should at least throw a warning if something is unusual. Also I don't want to blow it up, I want to reduce code if anything. — thesaint, Feb 27 '13 at 05:08
Actually hash is a functor not a function so maybe it's different. Anyway I just asked a [new question](http://stackoverflow.com/questions/15104760/c11-overloading-not-specializing-template-function-in-standard-namespace) about this if you're curious...in any case, it's probably not what's keeping your template from compiling on all compilers so I didn't bother to link back to this. — Stephen Lin, Feb 27 '13 at 05:24
Ok, apparently this is illegal, if you care about such things :D (you can always put it in another namespace and provide it explicitly, though) — Stephen Lin, Feb 27 '13 at 05:47
@thesaint: Nah, I've routinely copied Boost's five-line `hash_combine` and used to to hash all sorts of stuff. This is very straight-forward. I think it was simply forgotten in the standard. — Kerrek SB, Feb 27 '13 at 09:37
@KerrekSB, it wasn't forgotten, it was proposed for TR1 but the proposal wasn't mature enough, and when http://cplusplus.github.com/LWG/lwg-closed.html#1317 was suggested for C++0x it was too late for new features so we voted it "NAD Future" — Jonathan Wakely, Feb 27 '13 at 13:29
@StephenLin, it is a specialization not a new template, the class template name is a _template-id_ of the form `hash>` so it must be a (partial) specialization, also it must be a specialization not a new template because you can't overload class templates. It's not legal because it doesn't involve a user-defined type. — Jonathan Wakely, Feb 27 '13 at 13:30
@JonathanWakely: Well this is a pretty useless statement. If anything provide reasons, links as to why and also what you would recommend. Additionally XOR is stable. If you use good hash functions, like the first 64 bits of SHA-160, then XOR is about the perfect way to mix hash values. So you comment definitely deserves elaboration... For the usual "stupid" hash functions a rotate-left about "Index * 13" bits or so before XORing might be a better choice. — thesaint, Feb 27 '13 at 18:20
@thesaint any tuple with an even number of identical elements, regardless of their values, will always hash to all 0's for one thing. — Stephen Lin, Feb 27 '13 at 18:35
(also, the same values in different order will hash the same.) — Stephen Lin, Feb 27 '13 at 18:44
@JonathanWakely: Interesting. Why wasn't it considered mature enough? It looks like a fairly straight-forward algorithm...? — Kerrek SB, Feb 27 '13 at 18:57
@StephenLin: Okay that might be an issue, depending on the application. The rotate-left-XOR should take care of that... — thesaint, Feb 27 '13 at 22:00
@thesaint, just XOR is not a good hash function because {1,2,3,4} collides with {4,3,2,1}, for example — Jonathan Wakely, Feb 28 '13 at 10:05
The above plan is undefined behavior. You can only specialize `template`s in `namespace std` that on user-defined types, or types that depend on user-defined types. The above fails this test. — Yakk - Adam Nevraumont, May 14 '14 at 14:16

score 3 · Answer 1 · answered Feb 27 '13 at 22:56

3

Quite obvious once you've seen it:

template<int N, typename THead, typename... TTail>
size_t operator()(Tuple value) const
{
  constexpr int Index = N - sizeof...(TTail) - 1;
  return hash<THead>()(std::get<Index>(value)) ^ operator()<N, TTail...>(value);
}

answered Feb 27 '13 at 22:56

Daniel Frey

52,317
11
110
168

LOL, yeah thanks.. This is what happens if you "cook" a solution until it works. I've applied it above! – thesaint Feb 27 '13 at 23:34
Another one: You should take the parameter as `const Tuple& value` instead of creating copies all the time in all three places. – Daniel Frey Feb 27 '13 at 23:50
Well it depends. But it probably won't hurt in this case either. C++11 has the new idiom, to avoid "const&" and use "pass-by-value" instead. Hmm move-semantics. LOL I don't know why anymore. Move-semantics doesn't really seem to solve this either. Seems like a combination. Move-semantics for RValues and compiler internal optimizations for LValues. To me using "const&" seems like a premature optimization, unless you need inheritence. – thesaint Feb 28 '13 at 00:00
1

Nope, your above code (`Tuple value`) doesn't move anything when your functions call each other. It creates copies. Your only hope is the optimizer and I wouldn't rely on that. – Daniel Frey Feb 28 '13 at 00:07
I agree with you, the semantic is to copy... But people often tend to underestimate todays compilers. I optimize O(1) only when the profiler tells me so ;). Of course a STD library author has to think differently. – thesaint Feb 28 '13 at 00:16
1

The above is a symmetric hash, which means `make_tuple( 0, 0 )` and `make_tuple( 1, 1 )` both have a hash of `0`. Second, specializing `hash` in `std` for `std::tuple< Ts... >` is undefined behavior, as you can only specialize `template`s in `namespace std` for user-defined types or types that depend on user-defined types. – Yakk - Adam Nevraumont May 14 '14 at 14:15
@Yakk You are right about the symmetric part, which can easily improved by using some code (`hash_combine`) from [this answer](http://stackoverflow.com/a/19740245/2073257) I gave to another question. For the "undefined behaviour" part: No, user-defined does not mean outside of namespace `std`, it just means it's a class. See the comments on the answer I linked. – Daniel Frey May 14 '14 at 15:13
@DanielFrey you asserted types (like `std::vector`) in namespace `std` are UDT, but I did not see that being discussed. Just asserted. You seem to think that `user-defined type` means `non-fundamental type`, and I cannot find justification in the standard for that. I have just searched it (the current draft) for 'user-defined type' and 'user-defined' and have found no clause that would imply `std::vector` is a user-defined type. Every use of 'user-defined type' seems to make perfect sense if it only refers to types users (and `std::` is not a user) define. Did I miss something? – Yakk - Adam Nevraumont May 14 '14 at 15:29
@DanielFrey Asked a question about this issue [here](http://stackoverflow.com/questions/23659222/is-stdvector-a-user-defined-type) – Yakk - Adam Nevraumont May 14 '14 at 15:39

My std::hash for std::tuples... Any improvements?

1 Answers1

Linked