[cfe-commits] RFC - Fix std::hash bugs and remove reinterpret_cast<>

[John McCall]

On Dec 2, 2011, at 1:00 PM, Dave Zarzycki wrote:

> This fixes a bug where garbage is being read during std::hash of float on 64-bit platforms.
> This also removes all of the calls to reinterpret_cast<>.
> 
> Dave Z.
> 
> 
> diff --git a/include/functional b/include/functional
> index 17a36cc87ad..0300b68eae8 100644
> --- a/include/functional
> +++ b/include/functional
> @@ -1921,11 +1921,17 @@ struct _LIBCPP_VISIBLE hash<long long>
>     _LIBCPP_INLINE_VISIBILITY
>     size_t operator()(long long __v) const _NOEXCEPT
>     {
> -        size_t __r = 0;
> -        const size_t* const __p = reinterpret_cast<const size_t*>(&__v);
> -        for (unsigned __i = 0; __i < sizeof(argument_type)/sizeof(size_t); ++__i)
> -            __r ^= __p[__i];
> -        return __r;
> +        if (sizeof(long long) == sizeof(size_t))
> +            return __v;

<= should be fine.  Also, if you use an explicit cast to unsigned long long, you'll
suppress a warning;  it will also avoid a sign-extend in the unlikely case of
long long being smaller than size_t.

> +        union {
> +            long long __l;
> +            struct {
> +                int __a;
> +                int __b;
> +            } __s;
> +        } __u;
> +        __u.__l = __v;
> +        return __u.__s.__a ^ __u.__s.__b;

I understand that true C portability is hard, but I'm sure we can do better than this.

> @@ -1984,11 +2010,21 @@ struct _LIBCPP_VISIBLE hash<long double>
>     {
>         if (__v == 0)
>             return 0;
> -        size_t __r = 0;
> -        const size_t* const __p = reinterpret_cast<const size_t*>(&__v);
> -        for (unsigned __i = 0; __i < sizeof(argument_type)/sizeof(size_t); ++__i)
> -            __r ^= __p[__i];
> -        return __r;
> +        if (sizeof(double) == sizeof(size_t)) {
> +            union {
> +                double __f;
> +                size_t __d;
> +            } __u;
> +            __u.__f = __v;
> +            return __u.__d;
> +        } else {
> +            union {
> +                float __f;
> +                size_t __d;
> +            } __u;
> +            __u.__f = __v;
> +            return __u.__d;

I'm not sure that sizeof(double) != sizeof(size_t) implies that
sizeof(float) == sizeof(size_t). :)

John.