Skip to content

Commit

Permalink
is_integral_or_enum ❥ enum class ⇒ hashable enum class
Browse files Browse the repository at this point in the history
Summary:
As discussed in D18775 making AtomicOrdering an enum class makes it non-hashable, which shouldn't be the case. Hashing.h defines hash_value for all is_integral_or_enum, but type_traits.h's definition of is_integral_or_enum only checks for *inplicit* conversion to integral types which leaves enum classes out and is very confusing because is_enum is true for enum classes.

This patch:
  - Adds a check for is_enum when determining is_integral_or_enum.
  - Explicitly converts the value parameter in hash_value to handle enum class hashing.

Note that the warning at the top of Hashing.h still applies: each execution of the program has a high probability of producing a different hash_code for a given input. Thus their values are not stable to save or persist, and should only be used during the execution for the construction of hashing datastructures.

Reviewers: dberlin, chandlerc

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D18938

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@265879 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
jfbastien committed Apr 9, 2016
1 parent 030f43a commit 0b0b58f
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
3 changes: 2 additions & 1 deletion include/llvm/ADT/Hashing.h
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,8 @@ inline hash_code hash_integer_value(uint64_t value) {
template <typename T>
typename std::enable_if<is_integral_or_enum<T>::value, hash_code>::type
hash_value(T value) {
return ::llvm::hashing::detail::hash_integer_value(value);
return ::llvm::hashing::detail::hash_integer_value(
static_cast<typename std::underlying_type<T>::type>(value));
}

// Declared and documented above, but defined here so that any of the hashing
Expand Down
10 changes: 6 additions & 4 deletions include/llvm/Support/type_traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,12 @@ struct isPodLike<std::pair<T, U> > {
};

/// \brief Metafunction that determines whether the given type is either an
/// integral type or an enumeration type.
/// integral type or an enumeration type, including enum classes.
///
/// Note that this accepts potentially more integral types than is_integral
/// because it is based on merely being convertible implicitly to an integral
/// type.
/// because it is based on being implicitly convertible to an integral type.
/// Also note that enum classes aren't implicitly convertible to integral types,
/// the value may therefore need to be explicitly converted before being used.
template <typename T> class is_integral_or_enum {
typedef typename std::remove_reference<T>::type UnderlyingT;

Expand All @@ -67,7 +68,8 @@ template <typename T> class is_integral_or_enum {
!std::is_class<UnderlyingT>::value && // Filter conversion operators.
!std::is_pointer<UnderlyingT>::value &&
!std::is_floating_point<UnderlyingT>::value &&
std::is_convertible<UnderlyingT, unsigned long long>::value;
(std::is_enum<UnderlyingT>::value ||
std::is_convertible<UnderlyingT, unsigned long long>::value);
};

/// \brief If T is a pointer, just return it. If it is not, return T&.
Expand Down

0 comments on commit 0b0b58f

Please sign in to comment.