Re: [PATCH] D18938: is_integral_or_enum ❥ enum class ⇒ hashable enum class

JF Bastien via llvm-commits llvm-commits at lists.llvm.org
Sat Apr 9 13:08:36 PDT 2016


jfb added a comment.

In http://reviews.llvm.org/D18938#396364, @dberlin wrote:

> This looks better than the hackery i came up with, so LGTM :)
>
> One nit in case we want to care:
>
> +  return ::llvm::hashing::detail::hash_integer_value((uint64_t)value);
>
> So, you are guaranteed this value is either an enum, *or* convertible to
>  unsigned long long.
>
> But what is the enum is not an unsigned type?
>
> IE will this not still trigger on:
>
> enum class e2: int {};
>
> ?
>
> I know these are all just hash values, so who cares, but in practice, you
>  may just want to use an explicit cast to  std::underlying_type instead of
>  uint64_t.
>  or something.  Again, my C++ knowledge mostly died after C++98, so not sure
>  what the "right" answer is here.


I was being a bit lazy because the signature is `hash_code hash_integer_value(uint64_t value)`, but you're right using `underlying_type` is probably better if `hash_integer_value`'s input type were to change.

Fixed that.


http://reviews.llvm.org/D18938





More information about the llvm-commits mailing list