Bug 1443706 - Introduce ConstExprHashString(const char16_t*). r=jwalden draft
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 08 Mar 2018 10:27:14 +1100
changeset 766618 792d3a81e77ff1919781a1b8b73272b02c5bfe0c
parent 766617 12845d45a466b90ac319f0956ef1370017d0b999
push id102371
push usernnethercote@mozilla.com
push dateTue, 13 Mar 2018 06:01:15 +0000
reviewersjwalden
bugs1443706, 1443342, 1411469
milestone60.0a1
Bug 1443706 - Introduce ConstExprHashString(const char16_t*). r=jwalden This is a `constexpr` alternative to HashString(const char16_t*). We can't make HashString(const char16_t*) itself `constexpr` because HashUntilZero(const T*) isn't in a form that older compilers (like GCC 4.9 and MSVC 2015) allow to be made `constexpr`. (The trick to satisfying those compilers is to use recursion instead of iteration, to get the function into a single `return` statement.) This requires making a bunch of other functions `constexpr` as well. It also requires adding MOZ_{PUSH,POP}_DISABLE_INTEGRAL_CONSTANT_OVERFLOW_WARNING macros to avoid some MSVC weirdness. The introduction of RotateLeft5() partly undoes one of the patches from bug 1443342, but that's unavoidable. This change will help with static allocation of static atoms (bug 1411469). MozReview-Commit-ID: 7r3PnrQXb29
mfbt/HashFunctions.h
mfbt/WrappingOperations.h
--- a/mfbt/HashFunctions.h
+++ b/mfbt/HashFunctions.h
@@ -60,17 +60,24 @@ namespace mozilla {
 
 /**
  * The golden ratio as a 32-bit fixed-point value.
  */
 static const uint32_t kGoldenRatioU32 = 0x9E3779B9U;
 
 namespace detail {
 
-inline uint32_t
+MOZ_NO_SANITIZE_UNSIGNED_OVERFLOW
+constexpr uint32_t
+RotateLeft5(uint32_t aValue)
+{
+  return (aValue << 5) | (aValue >> 27);
+}
+
+constexpr uint32_t
 AddU32ToHash(uint32_t aHash, uint32_t aValue)
 {
   /*
    * This is the meat of all our hash routines.  This hash function is not
    * particularly sophisticated, but it seems to work well for our mostly
    * plain-text inputs.  Implementation notes follow.
    *
    * Our use of the golden ratio here is arbitrary; we could pick almost any
@@ -84,17 +91,17 @@ AddU32ToHash(uint32_t aHash, uint32_t aV
    *
    * The rotation length of 5 is also arbitrary, although an odd number is again
    * preferable so our hash explores the whole universe of possible rotations.
    *
    * Finally, we multiply by the golden ratio *after* xor'ing, not before.
    * Otherwise, if |aHash| is 0 (as it often is for the beginning of a
    * message), the expression
    *
-   *   mozilla::WrappingMultiply(kGoldenRatioU32, RotateBitsLeft(aHash, 5))
+   *   mozilla::WrappingMultiply(kGoldenRatioU32, RotateLeft5(aHash))
    *   |xor|
    *   aValue
    *
    * evaluates to |aValue|.
    *
    * (Number-theoretic aside: Because any odd number |m| is relatively prime to
    * our modulus (2**32), the list
    *
@@ -105,24 +112,24 @@ AddU32ToHash(uint32_t aHash, uint32_t aV
    *
    * It's also nice if |m| has large-ish order mod 2**32 -- that is, if the
    * smallest k such that m**k == 1 (mod 2**32) is large -- so we can safely
    * multiply our hash value by |m| a few times without negating the
    * multiplicative effect.  Our golden ratio constant has order 2**29, which is
    * more than enough for our purposes.)
    */
   return mozilla::WrappingMultiply(kGoldenRatioU32,
-                                   RotateLeft(aHash, 5) ^ aValue);
+                                   RotateLeft5(aHash) ^ aValue);
 }
 
 /**
  * AddUintptrToHash takes sizeof(uintptr_t) as a template parameter.
  */
 template<size_t PtrSize>
-inline uint32_t
+constexpr uint32_t
 AddUintptrToHash(uint32_t aHash, uintptr_t aValue)
 {
   return AddU32ToHash(aHash, static_cast<uint32_t>(aValue));
 }
 
 template<>
 inline uint32_t
 AddUintptrToHash<8>(uint32_t aHash, uintptr_t aValue)
@@ -168,17 +175,17 @@ AddToHash(uint32_t aHash, A* aA)
   return detail::AddUintptrToHash<sizeof(uintptr_t)>(aHash, uintptr_t(aA));
 }
 
 // We use AddUintptrToHash() for hashing all integral types.  8-byte integral types
 // are treated the same as 64-bit pointers, and smaller integral types are first
 // implicitly converted to 32 bits and then passed to AddUintptrToHash() to be hashed.
 template<typename T,
          typename U = typename mozilla::EnableIf<mozilla::IsIntegral<T>::value>::Type>
-MOZ_MUST_USE inline uint32_t
+MOZ_MUST_USE constexpr uint32_t
 AddToHash(uint32_t aHash, T aA)
 {
   return detail::AddUintptrToHash<sizeof(T)>(aHash, aA);
 }
 
 template<typename A, typename... Args>
 MOZ_MUST_USE uint32_t
 AddToHash(uint32_t aHash, A aArg, Args... aArgs)
@@ -208,16 +215,29 @@ HashUntilZero(const T* aStr)
 {
   uint32_t hash = 0;
   for (T c; (c = *aStr); aStr++) {
     hash = AddToHash(hash, c);
   }
   return hash;
 }
 
+// This is a `constexpr` alternative to HashUntilZero(const T*). It should
+// only be used for compile-time computation because it uses recursion.
+// XXX: once support for GCC 4.9 and MSVC 2015 is dropped, this function should
+// be removed and HashUntilZero(const T*) should be made `constexpr`.
+template<typename T>
+constexpr uint32_t
+ConstExprHashUntilZero(const T* aStr, uint32_t aHash)
+{
+  return !*aStr
+       ? aHash
+       : ConstExprHashUntilZero(aStr + 1, AddToHash(aHash, *aStr));
+}
+
 template<typename T>
 uint32_t
 HashKnownLength(const T* aStr, size_t aLength)
 {
   uint32_t hash = 0;
   for (size_t i = 0; i < aLength; i++) {
     hash = AddToHash(hash, aStr[i]);
   }
@@ -252,16 +272,31 @@ HashString(const unsigned char* aStr, si
 }
 
 MOZ_MUST_USE inline uint32_t
 HashString(const char16_t* aStr)
 {
   return detail::HashUntilZero(aStr);
 }
 
+// This is a `constexpr` alternative to HashString(const char16_t*). It should
+// only be used for compile-time computation because it uses recursion.
+//
+// You may need to use the
+// MOZ_{PUSH,POP}_DISABLE_INTEGRAL_CONSTANT_OVERFLOW_WARNING macros if you use
+// this function. See the comment above WrappingMultiply() for more detail.
+//
+// XXX: once support for GCC 4.9 and MSVC 2015 is dropped, this function should
+// be removed and HashString(const char16_t*) should be made `constexpr`.
+MOZ_MUST_USE constexpr uint32_t
+ConstExprHashString(const char16_t* aStr)
+{
+  return detail::ConstExprHashUntilZero(aStr, 0);
+}
+
 MOZ_MUST_USE inline uint32_t
 HashString(const char16_t* aStr, size_t aLength)
 {
   return detail::HashKnownLength(aStr, aLength);
 }
 
 /*
  * On Windows, wchar_t is not the same as char16_t, even though it's
--- a/mfbt/WrappingOperations.h
+++ b/mfbt/WrappingOperations.h
@@ -93,47 +93,37 @@ namespace detail {
 
 template<typename T>
 struct WrappingMultiplyHelper
 {
 private:
   using UnsignedT = typename MakeUnsigned<T>::Type;
 
   MOZ_NO_SANITIZE_UNSIGNED_OVERFLOW
-  static UnsignedT
+  static constexpr UnsignedT
   multiply(UnsignedT aX, UnsignedT aY)
   {
-  // |mozilla::WrappingMultiply| isn't constexpr because MSVC warns about well-
-  // defined unsigned integer overflows that may happen here.
-  // https://msdn.microsoft.com/en-us/library/4kze989h.aspx  And constexpr
-  // seems to cause the warning to be emitted at |WrappingMultiply| call *sites*
-  // instead of here, so these #pragmas are ineffective.
-  //
-  // https://stackoverflow.com/questions/37658794/integer-constant-overflow-warning-in-constexpr
-  //
-  // If/when MSVC fix this bug, we should make these functions constexpr.
-
     // Begin with |1U| to ensure the overall operation chain is never promoted
     // to signed integer operations that might have *signed* integer overflow.
     return static_cast<UnsignedT>(1U * aX * aY);
   }
 
-  static T
+  static constexpr T
   toResult(UnsignedT aX, UnsignedT aY)
   {
     // We could always return WrapToSigned and rely on unsigned conversion
     // undoing the wrapping when |T| is unsigned, but this seems clearer.
     return IsSigned<T>::value
            ? WrapToSigned(multiply(aX, aY))
            : multiply(aX, aY);
   }
 
 public:
   MOZ_NO_SANITIZE_UNSIGNED_OVERFLOW
-  static T compute(T aX, T aY)
+  static constexpr T compute(T aX, T aY)
   {
     return toResult(static_cast<UnsignedT>(aX), static_cast<UnsignedT>(aY));
   }
 };
 
 } // namespace detail
 
 /**
@@ -170,19 +160,45 @@ public:
  *   WrappingMultiply(int8_t(16), int8_t(24)) is -128 ((384 mod 2**8) - 2**8);
  *   WrappingMultiply(int8_t(16), int8_t(255)) is -16 ((4080 mod 2**8) - 2**8).
  *
  * There is no ready equivalent to this operation in C++, as applying C++
  * multiplication to signed integer types in ways that trigger overflow has
  * undefined behavior.  However, it's how multiplication *tends* to behave with
  * most compilers in most situations, even though it's emphatically not required
  * to do so.
+ *
+ * Note: MSVC warns about well-defined unsigned integer overflows that may
+ * happen here when this function is used in a constexpr expression.
+ * (https://msdn.microsoft.com/en-us/library/4kze989h.aspx has details about
+ * the warning.) Furthermore, the warnings are emitted not for this code, but
+ * seemingly for the callsite that begins the constexpr expression. So we can't
+ * use pragmas to suppress the warning here, but must do it at those call
+ * sites. Use the MOZ_{PUSH,POP}_DISABLE_INTEGRAL_CONSTANT_OVERFLOW_WARNING
+ * macros for this.
+ *
+ * https://stackoverflow.com/questions/37658794/integer-constant-overflow-warning-in-constexpr
+ *
+ * If/when MSVC fix this bug, we can remove the
+ * MOZ_{PUSH,POP}_DISABLE_INTEGRAL_CONSTANT_OVERFLOW_WARNING macros.
  */
 template<typename T>
-inline T
+constexpr T
 WrappingMultiply(T aX, T aY)
 {
   return detail::WrappingMultiplyHelper<T>::compute(aX, aY);
 }
 
 } /* namespace mozilla */
 
+// See the comment above WrappingMultiply() for an explanation of these.
+#ifdef _MSC_VER
+#define MOZ_PUSH_DISABLE_INTEGRAL_CONSTANT_OVERFLOW_WARNING \
+  __pragma(warning(push)) \
+  __pragma(warning(disable:4307))
+#define MOZ_POP_DISABLE_INTEGRAL_CONSTANT_OVERFLOW_WARNING \
+  __pragma(warning(pop))
+#else
+#define MOZ_PUSH_DISABLE_INTEGRAL_CONSTANT_OVERFLOW_WARNING
+#define MOZ_POP_DISABLE_INTEGRAL_CONSTANT_OVERFLOW_WARNING
+#endif
+
 #endif /* mozilla_WrappingOperations_h */