Re: PATCH: TR1 unordered associative containers

[Geoffrey Keating <geoffk at geoffk dot org>]

Matt Austern <austern@apple.com> writes:

Hi Matt,

Looks pretty neat!  However,

> +   tr1_hashtable_define_trivial_hash(float);
> +   tr1_hashtable_define_trivial_hash(double);
> +   tr1_hashtable_define_trivial_hash(long double);

cast-to-int probably isn't a very good hash function for
floating-point values...

> +   #undef tr1_hashtable_define_trivial_hash
> +
> +   template <typename T>
> +     struct hash<T*> {
> +       std::size_t operator()(T* p) const {
> + 	return reinterpret_cast<std::size_t>(p);
> +       }
> +     };
> +
> +   // ??? We can probably find a better hash function than this (i.e.
> one
> +   // that vectorizes better and that produces a more uniform
> distribution).
> +
> +   // XXX String hash probably shouldn't be an inline member function,
> +   // since it's nontrivial.  Once we have the framework for TR1 .cc
> +   // files, this should go in one.

And like this function, there's a problem: once you put this in an
inline function in a shipping compiler, even once, you're stuck with
it forever; that is, you can't change the function after that point.
So I think it'd be best to put them in a .cc file in the first commit.

> + // XXX This is a hack.  prime_rehash_policy's member functions, and
> + // certainly the list of primes, should be defined in a .cc file.
> + // We're temporarily putting them in a header because we don't have a
> + // place to put TR1 .cc files yet.  There's no good reason for any of
> + // prime_rehash_policy's member functions to be inline, and there's
> + // certainly no good reason for X<> to exist at all.

Likewise, this, and that'd be really annoying.  Especially if you
later wanted to extend the list of primes.