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