[modules] Use `HashBuilder` and `MD5` for the module hash.

[arames]

Summary

Per the comments, hash_code values "are not stable to save or
persist", so are unsuitable for the module hash, which must persist
across compilations for the implicit module hashes to match. Note that
in practice, today, hash_code are stable. But this is an
implementation detail, with a clear FIXME indicating we should switch
to a per-execution seed.

The stability of MD5 also allows modules cross-compilation use-cases.
The size_t underlying storage for hash_code varying across platforms
could cause mismatching hashes when cross-compiling from a 64bit
target to a 32bit target.

Note that native endianness is still used for the hash computation. So hashes
will differ between platforms of different endianness.

[jroelofs]

why do module hashes need to be stable when cross-compiling?

[dexonsmith]

In D102943#2774732, @jroelofs wrote:

why do module hashes need to be stable when cross-compiling?

IIUC, the use case is cross-compiling when building the modules, which are then sent to the target to use when compiling other things.

[pcc]

Isn't the bug here that module hashing is using hash_code? So shouldn't the correct fix be to use a specific hashing algorithm for module hashes?

[dexonsmith]

In D102943#2775115, @pcc wrote:

Isn't the bug here that module hashing is using hash_code? So shouldn't the correct fix be to use a specific hashing algorithm for module hashes?

Yeah, I tend to agree. I thought modules already used MD5, but maybe just for the AST signature.

[arames]

In D102943#2775131, @dexonsmith wrote:

In D102943#2775115, @pcc wrote:

Isn't the bug here that module hashing is using hash_code? So shouldn't the correct fix be to use a specific hashing algorithm for module hashes?

Yeah, I tend to agree. I thought modules already used MD5, but maybe just for the AST signature.

For reference, the issue I am trying to fix stems from the hash computed as part of CompilerInvocation::getModuleHash().

Reading the doc around llvm::hash_code, my first reaction was indeed that modules may have to use a different hash.
I thought the current proposal was something to consider, because:

1. Practically, everything is done on uint64_t, so I would expect switching from size_t to uint64_t to have little to no impact on non-64bit targets.
2. The current modules code kind of already assumes hash_code is stable over different executions. Plain simple use-case with implicit modules would be severely deficient if the hash was not stable across executions. To be fair, the assumption is different from the one I am trying to fix of 32bit vs 64bit.

Alternatively, we could introduce something along the lines of llvm::stable_hash_code (other name suggestions welcome).
We can probably restructure the code with very few changes. (Really, it looks like we only need a configurable storage type, and allow skipping the execution seed part.)

Would you think this is a better approach ? Or do you have something else to suggest ?

Cheers

[arames]

In D102943#2775099, @dexonsmith wrote:

In D102943#2774732, @jroelofs wrote:

why do module hashes need to be stable when cross-compiling?

IIUC, the use case is cross-compiling when building the modules, which are then sent to the target to use when compiling other things.

Yes, that's right.
In particular, we are talking about the hash used for for (prebuilt) implicit modules paths.

[arames]

Introduce and use stable_hash_code instead of modifying hash_code.

[arames]

The early commits are missing from the PR. Looking out to do this with arc.

[arames]

Diff against the parent commit.

[arames]

This new version is an attempt to have modules not rely on llvm::hash_code, but on a new llvm::stable_hash_code.
I understand modifying ADT/Hashing.h is sensitive, so maybe we need to discuss the high-level approach first.

[dexonsmith]

In D102943#2786015, @arames wrote:

This new version is an attempt to have modules not rely on llvm::hash_code, but on a new llvm::stable_hash_code.
I understand modifying ADT/Hashing.h is sensitive, so maybe we need to discuss the high-level approach first.

Just seeing the update here, but we already chatted offline. @arames is going to try using llvm::MD5, which is used elsewhere for a stable hash in modules (for the AST signature), rather than introducing stable_hash_*.

[arames]

Use llvm::MD5.

[arames]

clang/include/clang/Basic/Sanitizers.h

81

An alternative to having those in class would be to have helpers directly in CompilerInvocation.cpp. I decided to keep them here for two reasons:

1. Keep them next to hash_value, to make it clear they need to be updated as well.
2. At least this class does not have all elements going into the hash public.

[arames]

Fix detail namespace.

[dexonsmith]

This looks much cleaner; thanks for the update!

I'm not sure this has decided whether it's writing special case code for building a CompilerInvocation's module context hash, or creating (and using) a new generalized hash builder.

If the latter, I think it'd be worth splitting the hash builder out and landing it separately (in LLVM), then adopting it here; I think that's what you were doing with stable_hash_code (although that had the downsides of reifying city hash and not reusing the Hasher tools we have). This could take a few rounds of review to get the design right / get consensus on... not sure what you're able to invest. A few design questions I think you'd want to think about:

• What's the interaction between hash_value and (e.g.) updateHash? (It'd be nice if implementing updateHash gave you an implementation of hash_value for free.)
• Should updateHash be templated on the HasherT, or use type erasure? (E.g., a stripped down HashBuilder type could have a single member that was llvm::unique_function<void(ArrayRef<uint8_t>)>, and all the updateHash() functions are just free functions)
• Should the hash builder reference a HasherT, or own it?
• What are the API affordances for hashing structure? (E.g., ensuring that calls to updateHash with "a" and "bc" give a different hash than "ab" and "c".)

On the other hand, if you're not going to have time to generalize this, I'd suggest simplifying the code a bit, merging the "detail" subclass into the builder type. I think ModuleContextHashBuilder might be a better name than ModuleHash; I think in clangBasic, rather than clangSerialization, since it could be used for things other than serialization and doesn't really have any dependencies there.

In either case, there's a decent chunk of new code, and I think you should add some unit test coverage (a new file in llvm/unittests/Support/, in a separate patch, and/or clang/unittests/ (depending on direction)).

A have a bunch of comments inline, some which would only apply depending on which direction you're going with this.

@Bigcheese and @jansvoboda11 , any thoughts?

clang/include/clang/Serialization/ModuleHash.h

19–22

I think this builder is an interesting idea, and could be generally useful.

• It'd fit better in llvm/Support I think than hidden here.
• Taking the hasher by reference might be more flexible. Clients can ask the builder to do some of the hashing, but do some on their own.

36

I don't think the names HashValueT and Hash are quite right (I tend to think of "hash" as a value as well).

• Maybe HasherT instead of HashValueT?
• Maybe HashBuilder instead of Hash?

Nit: I think (?) we more often use class for type template parameters than typename.

43

For a builder interface, it might be more convenient to take the hasher by reference.

47–51

Because this doesn't hash the distance of the range, it won't see a difference between {{0}, {1, 2}} and {{0, 1}, {2}}. Sometimes that's fine, but sometimes it isn't. I worry about generalizing this... unless the onus is on the caller to hash the size when appropriate?

54–57

I'm not sure we want it to be easy to hash pointers by-value. That's appropriate for an in-memory hash table, but not much else.

77–83

Is there a way of reducing the boilerplate for these update_impl functions, which are currently types? Could they maybe be free functions?

If you hit trouble with using std::enable_if_t in a function template, I seem to remember needing to use something like this in the past:

std::enable_if_t<whatever<T>::value, bool> = false

96–104

I wonder if this should also hash the size of the StringRef. Maybe it should also be called updateRange.

MD5/SHA1/etc. hashers have the perspective that they're seeing a flat stream of bytes, and I think this streams in the bytes from the string directly. I think the right way to model hashing the context hash is as a serialization, and serializing a string ref requires serializing the size before the content, so that the deserializer knows how big the string is.

clang/lib/Frontend/CompilerInvocation.cpp

4483–4484

I'm not sure MD5 is cryptographically sound either, so it probably makes sense to leave this FIXME in / update it.

4580

Do we just want 64 bits, or do we want the full hash?

clang/lib/Serialization/ModuleFileExtension.cpp

16–18

Seems like this could stay in the .cpp, not sure if moving it to the header is important (or could be done separately to make the functionality changes more obvious).

llvm/include/llvm/Support/VersionTuple.h

164–167

Could this be declared as a friend, similar to hash_value?

Note that if the new builder code doesn't live in llvm/, probably this updateHash free function should be an implementation detail in the CompilerInvocation / ModuleHash code.

[arames]

Now using the HashBuilder interface.

[arames]

I still need to go through earlier comments.

[arames]

clang/lib/Frontend/CompilerInvocation.cpp

4580

My guess is either would be fine. I thought combining the two parts was simple enough.

[arames]

clang/include/clang/Serialization/ModuleFileExtension.h

90

This obviously needs to be fixed.

[arames]

Fix to native endianness.

[arames]

clang/include/clang/Basic/ObjCRuntime.h

485

I have added these in the same line as existing hash_value functions. The idea being others may also need to hash those objects.
Let me know if you would rather move some/all of these locally in CompilerInvocation.cpp.

[dexonsmith]

A couple of suggestions inline, but this basically LGTM.

@jansvoboda11 @Bigcheese : can you take a look as well and confirm this looks reasonable from your perspectives?

clang/include/clang/Basic/ObjCRuntime.h

485

Seems reasonable to me.

An option to consider (maybe in a separate commit?) would be to add:

class HashCodeHasher {
public:
  void update(ArrayRef<uint8_t> Bytes) {
    llvm::hash_code BytesCode = llvm::hash_value(Bytes);
    Code = Code ? BytesCode : llvm::hash_combine(*Code, BytesCode);
  }
  Optional<llvm::hash_code> Code;
};
template <class T,
          std::enable_if_t<is_hashable<T>::value, bool> = true>
llvm::hash_code hash_value(const T &Value) {
  HashBuilder<HashCodeHasher, support::endianness::native> Builder;
  Builder.add(Value);
  return *Builder.Code;
}

Then objects that support HashBuilder also support llvm::hash_value without extra code...

clang/lib/Frontend/CompilerInvocation.cpp

4475–4477

I'd simplify:

// FIXME: Consider using SHA1 instead of MD5.

llvm/include/llvm/Support/HashBuilder.h

167 ↗

(On Diff #369489)

Please fix in a separate commit (no need for a review IMO).

[arames]

clang/include/clang/Basic/ObjCRuntime.h

485

I created https://reviews.llvm.org/D109024 for this.
If we decide to go for that mechanism, whichever PR lands first, I can update the other to use the new mechanism.

clang/lib/Frontend/CompilerInvocation.cpp

4475–4477

Do you think we should switch to SHA1 now ?
I picked MD5 at random amongst the available hashes.

[arames]

Address review comments.

[jansvoboda11]

This looks good to me.

clang/include/clang/Basic/ObjCRuntime.h

friend bool operator!=(const ObjCRuntime &left, const ObjCRuntime &right) {
     return !(left == right);
   }
 
   friend llvm::hash_code hash_value(const ObjCRuntime &OCR) {
     return llvm::hash_combine(OCR.getKind(), OCR.getVersion());
   }
 };
 
 raw_ostream &operator<<(raw_ostream &out, const ObjCRuntime &value);
 
+template <typename HashT> void updateHash(HashT &Hash, const ObjCRuntime &OCR) {
+  Hash.update(OCR.getKind());
+  Hash.update(OCR.getVersion());
+}
 } // namespace clang
 
 #endif // LLVM_CLANG_BASIC_OBJCRUNTIME_H

clang/include/clang/Basic/Sanitizers.h

public:
   }
 
   bool isPowerOf2() const {
     return countPopulation() == 1;
   }
 
   llvm::hash_code hash_value() const;
 
+  template <typename HashT> void updateHash(HashT &Hash) const {
+    Hash.updateRange(&maskLoToHigh[0], &maskLoToHigh[0] + kNumElem);
+  }
+
   constexpr explicit operator bool() const {
     return maskLoToHigh[0] || maskLoToHigh[1];
   }
 
   constexpr bool operator==(const SanitizerMask &V) const {
     return maskLoToHigh[0] == V.maskLoToHigh[0] &&
            maskLoToHigh[1] == V.maskLoToHigh[1];
   }
friend constexpr inline SanitizerMask operator|(SanitizerMask a,
                                                   const SanitizerMask &b) {
     return SanitizerMask(a.maskLoToHigh[0] | b.maskLoToHigh[0],
                          a.maskLoToHigh[1] | b.maskLoToHigh[1]);
   }
 };
 
 // Declaring in clang namespace so that it can be found by ADL.
 llvm::hash_code hash_value(const clang::SanitizerMask &Arg);
+template <typename HashT>
+void updateHash(HashT &Hash, const clang::SanitizerMask &Arg) {
+  Arg.updateHash(Hash);
+}
 
 // Define the set of sanitizer kinds, as well as the set of sanitizers each
 // sanitizer group expands into.
 struct SanitizerKind {
   // Assign ordinals to possible values of -fsanitize= flag, which we will use
   // as bit positions.
   enum SanitizerOrdinal : uint64_t {
 #define SANITIZER(NAME, ID) SO_##ID,

clang/include/clang/Lex/HeaderSearchOptions.h

void AddPrebuiltModulePath(StringRef Name) {
     PrebuiltModulePaths.push_back(std::string(Name));
   }
 };
 
 inline llvm::hash_code hash_value(const HeaderSearchOptions::Entry &E) {
   return llvm::hash_combine(E.Path, E.Group, E.IsFramework, E.IgnoreSysRoot);
 }
 
+template <typename HashT>
+void updateHash(HashT &Hash, const HeaderSearchOptions::Entry &E) {
+  Hash.update(E.Path);
+  Hash.update(E.Group);
+  Hash.update(E.IsFramework);
+  Hash.update(E.IgnoreSysRoot);
+}
+
 inline llvm::hash_code
 hash_value(const HeaderSearchOptions::SystemHeaderPrefix &SHP) {
   return llvm::hash_combine(SHP.Prefix, SHP.IsSystemHeader);
 }
 
+template <typename HashT>
+void updateHash(HashT &Hash,
+                const HeaderSearchOptions::SystemHeaderPrefix &SHP) {
+  Hash.update(SHP.Prefix);
+  Hash.update(SHP.IsSystemHeader);
+}
 } // namespace clang
 
 #endif // LLVM_CLANG_LEX_HEADERSEARCHOPTIONS_H

clang/include/clang/Serialization/ModuleFileExtension.h

 //===-- ModuleFileExtension.h - Module File Extensions ----------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_SERIALIZATION_MODULEFILEEXTENSION_H
 #define LLVM_CLANG_SERIALIZATION_MODULEFILEEXTENSION_H
 
+#include "clang/Serialization/ModuleHash.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Support/ExtensibleRTTI.h"
 #include <memory>
 #include <string>
 
 namespace llvm {
 class BitstreamCursor;
 class BitstreamWriter;
public:
   static char ID;
 
   virtual ~ModuleFileExtension();
 
   /// Retrieves the metadata for this module file extension.
   virtual ModuleFileExtensionMetadata getExtensionMetadata() const = 0;
 
   /// Hash information about the presence of this extension into the
-  /// module hash code.
+  /// module hash.
   ///
-  /// The module hash code is used to distinguish different variants
+  /// The module hash is used to distinguish different variants
   /// of a module that are incompatible. If the presence, absence, or
   /// version of the module file extension should force the creation
   /// of a separate set of module files, override this method to
   /// combine that distinguishing information into the module hash
   /// code.
   ///
-  /// The default implementation of this function simply returns the
-  /// hash code as given, so the presence/absence of this extension
-  /// does not distinguish module files.
-  virtual llvm::hash_code hashExtension(llvm::hash_code c) const;
+  /// The default implementation of this function does not update the hash in
+  /// any way. So the presence/absence of this extension does not distinguish
+  /// module files.
+  virtual void hashExtension(ModuleHash &Hash) const {}
 
   /// Create a new module file extension writer, which will be
   /// responsible for writing the extension contents into a particular
   /// module file.
   virtual std::unique_ptr<ModuleFileExtensionWriter>
   createExtensionWriter(ASTWriter &Writer) = 0;
 
   /// Create a new module file extension reader, given the

clang/include/clang/Serialization/ModuleHash.h

+//===-- ModuleHash.h - Module Hash ------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_SERIALIZATION_MODULEHASH_H
+#define LLVM_CLANG_SERIALIZATION_MODULEHASH_H
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/MD5.h"
+#include <string>
+
+namespace module_hash {
+namespace detail {
+/// Wrapper interface around a hash type.
+/// The only methods expected from the hash type are:
+/// * a default constructor
+/// * void update(llvm::ArrayRef<uint8_t>)
+///
+/// Various types are forwarded to the canonical version of
+/// `HashValueT::update(llvm::ArrayRef<uint8_t>)`. Types requiring particular
+/// hashing behavior can specify it in a similar way to `llvm::hash_code`, by
+/// declaring
+///
+/// ```
+/// template <typename HashT>
+/// void updateHash(HashT &Hash, const ValueType &Value) {
+///   Hash.update(Value.member1);
+///   Hash.update(Value.member2);
+/// }
+/// ```
+template <typename HashValueT> class Hash {
+private:
+  template <typename T, typename E = void> struct update_impl {
+    update_impl(Hash &H, T Value) { updateHash(H, Value); }
+  };
+
+public:
+  HashValueT HashValue;
+
+  template <typename T> void update(T Value) { update_impl<T>(*this, Value); }
+
+  template <typename InputIteratorT>
+  void updateRange(InputIteratorT first, InputIteratorT last) {
+    for (auto It = first; It != last; ++It)
+      update(*It);
+  }
+
+private:
+  template <typename T>
+  struct is_hashable_data
+      : std::integral_constant<bool, llvm::is_integral_or_enum<T>::value ||
+                                         std::is_pointer<T>::value> {};
+
+public:
+  template <typename T>
+  typename std::enable_if<is_hashable_data<T>::value>::type
+  update(const std::initializer_list<T> &Value) {
+    update(llvm::ArrayRef<T>(Value.begin(), Value.size()));
+  }
+
+private:
+  // `void HashValueT::update(llvm::ArrayRef<uint8_t>)` is the canonical
+  // expected method to update the hash.
+  template <> struct update_impl<llvm::ArrayRef<uint8_t>> {
+    update_impl(Hash &H, llvm::ArrayRef<uint8_t> Value) {
+      H.HashValue.update(Value);
+    }
+  };
+
+  // Plain hashable data is forwarded to `llvm::ArrayRef<uint8_t>`.
+  template <typename T>
+  struct update_impl<T,
+                     typename std::enable_if<is_hashable_data<T>::value>::type>
+      : update_impl<llvm::ArrayRef<uint8_t>> {
+    update_impl(Hash &H, T value)
+        : update_impl<llvm::ArrayRef<uint8_t>>(
+              H, llvm::ArrayRef<uint8_t>(reinterpret_cast<uint8_t *>(&value),
+                                         sizeof(value))) {}
+  };
+
+  // Forward `llvm::ArrayRef<T>` to `llvm::ArrayRef<uint8_t>`.
+  template <typename T>
+  struct update_impl<llvm::ArrayRef<T>> : update_impl<llvm::ArrayRef<uint8_t>> {
+    update_impl(Hash &H, llvm::ArrayRef<T> Value)
+        : update_impl<llvm::ArrayRef<uint8_t>>(
+              H, llvm::ArrayRef<uint8_t>(
+                     reinterpret_cast<const uint8_t *>(Value.data()),
+                     Value.size() * sizeof(T))) {}
+  };
+
+  // Forward `llvm::StringRef` to `llvm::ArrayRef<uint8_t>`.
+  template <>
+  struct update_impl<llvm::StringRef> : update_impl<llvm::ArrayRef<uint8_t>> {
+    update_impl(Hash &H, llvm::StringRef Value)
+        : update_impl<llvm::ArrayRef<uint8_t>>(
+              H, llvm::ArrayRef<uint8_t>(
+                     reinterpret_cast<const uint8_t *>(Value.data()),
+                     Value.size())) {}
+  };
+
+  // Forward `std::string` to `llvm::StringRef`.
+  template <> struct update_impl<std::string> : update_impl<llvm::StringRef> {
+    update_impl(Hash &H, std::string Value)
+        : update_impl<llvm::StringRef>(H, Value) {}
+  };
+};
+} // namespace detail
+} // namespace module_hash
+
+struct ModuleHash : public module_hash::detail::Hash<llvm::MD5> {
+  uint64_t getValue() {
+    llvm::MD5::MD5Result HashResult;
+    HashValue.final(HashResult);
+    return HashResult.low();
+  }
+};
+
+#endif // LLVM_CLANG_FRONTEND_MODULEHASH_H

clang/lib/Frontend/CompilerInvocation.cpp

 #include "clang/Frontend/PreprocessorOutputOptions.h"
 #include "clang/Frontend/TextDiagnosticBuffer.h"
 #include "clang/Frontend/Utils.h"
 #include "clang/Lex/HeaderSearchOptions.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteOptions.h"
 #include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Serialization/ModuleFileExtension.h"
+#include "clang/Serialization/ModuleHash.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/CachedHashString.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/FloatingPointMode.h"
-#include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
return RoundTrip(
         return CreateFromArgsImpl(Invocation, CommandLineArgs, Diags, Argv0);
       },
       [](CompilerInvocation &Invocation, SmallVectorImpl<const char *> &Args,
          StringAllocator SA) { Invocation.generateCC1CommandLine(Args, SA); },
       Invocation, DummyInvocation, CommandLineArgs, Diags, Argv0);
 }
 
 std::string CompilerInvocation::getModuleHash() const {
   // Note: For QoI reasons, the things we use as a hash here should all be
   // dumped via the -module-info flag.
-  using llvm::hash_code;
-  using llvm::hash_value;
-  using llvm::hash_combine;
-  using llvm::hash_combine_range;
+  ModuleHash Hash;
 
   // Start the signature with the compiler version.
-  // FIXME: We'd rather use something more cryptographically sound than
-  // CityHash, but this will do for now.
-  hash_code code = hash_value(getClangFullRepositoryVersion());
+  Hash.update(getClangFullRepositoryVersion());
 
   // Also include the serialization version, in case LLVM_APPEND_VC_REV is off
   // and getClangFullRepositoryVersion() doesn't include git revision.
-  code = hash_combine(code, serialization::VERSION_MAJOR,
-                      serialization::VERSION_MINOR);
+  Hash.update({serialization::VERSION_MAJOR, serialization::VERSION_MINOR});
 
   // Extend the signature with the language options
-#define LANGOPT(Name, Bits, Default, Description) \
-   code = hash_combine(code, LangOpts->Name);
-#define ENUM_LANGOPT(Name, Type, Bits, Default, Description) \
-  code = hash_combine(code, static_cast<unsigned>(LangOpts->get##Name()));
+#define LANGOPT(Name, Bits, Default, Description) Hash.update(LangOpts->Name);
+#define ENUM_LANGOPT(Name, Type, Bits, Default, Description)                   \
+  Hash.update(static_cast<unsigned>(LangOpts->get##Name()));
 #define BENIGN_LANGOPT(Name, Bits, Default, Description)
 #define BENIGN_ENUM_LANGOPT(Name, Type, Bits, Default, Description)
 #include "clang/Basic/LangOptions.def"
 
   for (StringRef Feature : LangOpts->ModuleFeatures)
-    code = hash_combine(code, Feature);
+    Hash.update(Feature);
 
-  code = hash_combine(code, LangOpts->ObjCRuntime);
+  Hash.update(LangOpts->ObjCRuntime);
   const auto &BCN = LangOpts->CommentOpts.BlockCommandNames;
-  code = hash_combine(code, hash_combine_range(BCN.begin(), BCN.end()));
+  Hash.updateRange(BCN.begin(), BCN.end());
 
   // Extend the signature with the target options.
-  code = hash_combine(code, TargetOpts->Triple, TargetOpts->CPU,
-                      TargetOpts->TuneCPU, TargetOpts->ABI);
+  Hash.update(TargetOpts->Triple);
+  Hash.update(TargetOpts->CPU);
+  Hash.update(TargetOpts->TuneCPU);
+  Hash.update(TargetOpts->ABI);
   for (const auto &FeatureAsWritten : TargetOpts->FeaturesAsWritten)
-    code = hash_combine(code, FeatureAsWritten);
+    Hash.update(FeatureAsWritten);
 
   // Extend the signature with preprocessor options.
   const PreprocessorOptions &ppOpts = getPreprocessorOpts();
   const HeaderSearchOptions &hsOpts = getHeaderSearchOpts();
-  code = hash_combine(code, ppOpts.UsePredefines, ppOpts.DetailedRecord);
+  Hash.update(ppOpts.UsePredefines);
+  Hash.update(ppOpts.DetailedRecord);
 
   for (const auto &I : getPreprocessorOpts().Macros) {
     // If we're supposed to ignore this macro for the purposes of modules,
     // don't put it into the hash.
     if (!hsOpts.ModulesIgnoreMacros.empty()) {
       // Check whether we're ignoring this macro.
       StringRef MacroDef = I.first;
       if (hsOpts.ModulesIgnoreMacros.count(
               llvm::CachedHashString(MacroDef.split('=').first)))
         continue;
     }
 
-    code = hash_combine(code, I.first, I.second);
+    Hash.update(I.first);
+    Hash.update(I.second);
   }
 
   // Extend the signature with the sysroot and other header search options.
-  code = hash_combine(code, hsOpts.Sysroot,
-                      hsOpts.ModuleFormat,
-                      hsOpts.UseDebugInfo,
-                      hsOpts.UseBuiltinIncludes,
-                      hsOpts.UseStandardSystemIncludes,
-                      hsOpts.UseStandardCXXIncludes,
-                      hsOpts.UseLibcxx,
-                      hsOpts.ModulesValidateDiagnosticOptions);
-  code = hash_combine(code, hsOpts.ResourceDir);
+  Hash.update(hsOpts.Sysroot);
+  Hash.update(hsOpts.ModuleFormat);
+  Hash.update(hsOpts.UseDebugInfo);
+  Hash.update(hsOpts.UseBuiltinIncludes);
+  Hash.update(hsOpts.UseStandardSystemIncludes);
+  Hash.update(hsOpts.UseStandardCXXIncludes);
+  Hash.update(hsOpts.UseLibcxx);
+  Hash.update(hsOpts.ModulesValidateDiagnosticOptions);
+  Hash.update(hsOpts.ResourceDir);
 
   if (hsOpts.ModulesStrictContextHash) {
-    hash_code SHPC = hash_combine_range(hsOpts.SystemHeaderPrefixes.begin(),
-                                        hsOpts.SystemHeaderPrefixes.end());
-    hash_code UEC = hash_combine_range(hsOpts.UserEntries.begin(),
-                                       hsOpts.UserEntries.end());
-    code = hash_combine(code, hsOpts.SystemHeaderPrefixes.size(), SHPC,
-                        hsOpts.UserEntries.size(), UEC);
+    Hash.update(hsOpts.SystemHeaderPrefixes.size());
+    Hash.updateRange(hsOpts.SystemHeaderPrefixes.begin(),
+    Hash.update(hsOpts.UserEntries.size());
+    Hash.updateRange(hsOpts.UserEntries.begin(), hsOpts.UserEntries.end());
 
     const DiagnosticOptions &diagOpts = getDiagnosticOpts();
-    #define DIAGOPT(Name, Bits, Default) \
-      code = hash_combine(code, diagOpts.Name);
-    #define ENUM_DIAGOPT(Name, Type, Bits, Default) \
-      code = hash_combine(code, diagOpts.get##Name());
-    #include "clang/Basic/DiagnosticOptions.def"
-    #undef DIAGOPT
-    #undef ENUM_DIAGOPT
+#define DIAGOPT(Name, Bits, Default) Hash.update(diagOpts.Name);
+#define ENUM_DIAGOPT(Name, Type, Bits, Default)                                \
+  Hash.update(diagOpts.get##Name());
   }
 
   // Extend the signature with the user build path.
-  code = hash_combine(code, hsOpts.ModuleUserBuildPath);
+  Hash.update(hsOpts.ModuleUserBuildPath);
 
   // Extend the signature with the module file extensions.
   const FrontendOptions &frontendOpts = getFrontendOpts();
-  for (const auto &ext : frontendOpts.ModuleFileExtensions) {
-    code = ext->hashExtension(code);
-  }
+  for (const auto &ext : frontendOpts.ModuleFileExtensions)
+    ext->hashExtension(Hash);
 
   // When compiling with -gmodules, also hash -fdebug-prefix-map as it
   // affects the debug info in the PCM.
   if (getCodeGenOpts().DebugTypeExtRefs)
-    for (const auto &KeyValue : getCodeGenOpts().DebugPrefixMap)
-      code = hash_combine(code, KeyValue.first, KeyValue.second);
+    for (const auto &KeyValue : getCodeGenOpts().DebugPrefixMap) {
+      Hash.update(KeyValue.first);
+      Hash.update(KeyValue.second);
+    }
 
   // Extend the signature with the enabled sanitizers, if at least one is
   // enabled. Sanitizers which cannot affect AST generation aren't hashed.
   SanitizerSet SanHash = LangOpts->Sanitize;
   SanHash.clear(getPPTransparentSanitizers());
   if (!SanHash.empty())
-    code = hash_combine(code, SanHash.Mask);
+    Hash.update(SanHash.Mask);
 
-  return llvm::APInt(64, code).toString(36, /*Signed=*/false);
+  return llvm::APInt(64, Hash.getValue()).toString(36, /*Signed=*/false);
 }
 
 void CompilerInvocation::generateCC1CommandLine(
     SmallVectorImpl<const char *> &Args, StringAllocator SA) const {
   llvm::Triple T(TargetOpts->Triple);
 
   GenerateFileSystemArgs(FileSystemOpts, Args, SA);
   GenerateMigratorArgs(MigratorOpts, Args, SA);

clang/lib/Frontend/TestModuleFileExtension.h

TestModuleFileExtension(StringRef BlockName, unsigned MajorVersion,
                           unsigned MinorVersion, bool Hashed,
                           StringRef UserInfo)
       : BlockName(BlockName), MajorVersion(MajorVersion),
         MinorVersion(MinorVersion), Hashed(Hashed), UserInfo(UserInfo) {}
   ~TestModuleFileExtension() override;
 
   ModuleFileExtensionMetadata getExtensionMetadata() const override;
 
-  llvm::hash_code hashExtension(llvm::hash_code Code) const override;
+  void hashExtension(ModuleHash &Hash) const override;
 
   std::unique_ptr<ModuleFileExtensionWriter>
   createExtensionWriter(ASTWriter &Writer) override;
 
   std::unique_ptr<ModuleFileExtensionReader>
   createExtensionReader(const ModuleFileExtensionMetadata &Metadata,
                         ASTReader &Reader, serialization::ModuleFile &Mod,
                         const llvm::BitstreamCursor &Stream) override;
 
   std::string str() const;
 };
 
 } // end namespace clang
 
 #endif // LLVM_CLANG_FRONTEND_TESTMODULEFILEEXTENSION_H

clang/lib/Frontend/TestModuleFileExtension.cpp

 
 TestModuleFileExtension::~TestModuleFileExtension() { }
 
 ModuleFileExtensionMetadata
 TestModuleFileExtension::getExtensionMetadata() const {
   return { BlockName, MajorVersion, MinorVersion, UserInfo };
 }
 
-llvm::hash_code TestModuleFileExtension::hashExtension(
-                  llvm::hash_code Code) const {
+void TestModuleFileExtension::hashExtension(ModuleHash &Hash) const {
   if (Hashed) {
-    Code = llvm::hash_combine(Code, BlockName);
-    Code = llvm::hash_combine(Code, MajorVersion);
-    Code = llvm::hash_combine(Code, MinorVersion);
-    Code = llvm::hash_combine(Code, UserInfo);
+    Hash.update(BlockName);
+    Hash.update(MajorVersion);
+    Hash.update(MinorVersion);
+    Hash.update(UserInfo);
   }
-
-  return Code;
 }
 
 std::unique_ptr<ModuleFileExtensionWriter>
 TestModuleFileExtension::createExtensionWriter(ASTWriter &) {
   return std::unique_ptr<ModuleFileExtensionWriter>(new Writer(this));
 }
 
 std::unique_ptr<ModuleFileExtensionReader>

clang/lib/Serialization/ModuleFileExtension.cpp

 //===-- ModuleFileExtension.cpp - Module File Extensions ------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #include "clang/Serialization/ModuleFileExtension.h"
 #include "llvm/ADT/Hashing.h"
 using namespace clang;
 
 char ModuleFileExtension::ID = 0;
 
 ModuleFileExtension::~ModuleFileExtension() { }
 
-llvm::hash_code ModuleFileExtension::hashExtension(llvm::hash_code Code) const {
-  return Code;
-}
-
 ModuleFileExtensionWriter::~ModuleFileExtensionWriter() { }
 
 ModuleFileExtensionReader::~ModuleFileExtensionReader() { }

clang/unittests/Frontend/CompilerInvocationTest.cpp

 struct DummyModuleFileExtension
     : public llvm::RTTIExtends<DummyModuleFileExtension, ModuleFileExtension> {
   static char ID;
 
   ModuleFileExtensionMetadata getExtensionMetadata() const override {
     return {};
   };
 
-  llvm::hash_code hashExtension(llvm::hash_code Code) const override {
-    return {};
-  }
+  void hashExtension(ModuleHash &Hash) const override {}
 
   std::unique_ptr<ModuleFileExtensionWriter>
   createExtensionWriter(ASTWriter &Writer) override {
     return {};
   }
 
   std::unique_ptr<ModuleFileExtensionReader>
   createExtensionReader(const ModuleFileExtensionMetadata &Metadata,

llvm/include/llvm/Support/VersionTuple.h

public:
   /// \returns \c true if the string does not match the regular expression
   ///   [0-9]+(\.[0-9]+){0,3}
   bool tryParse(StringRef string);
 };
 
 /// Print a version number.
 raw_ostream &operator<<(raw_ostream &Out, const VersionTuple &V);
 
+template <typename HashT> void updateHash(HashT &Hash, const VersionTuple &V) {
+  Hash.update({V.getMajor(), V.getMinor().getValueOr(0),
+               V.getSubminor().getValueOr(0), V.getBuild().getValueOr(0)});
+}
+
 } // end namespace llvm
 #endif // LLVM_SUPPORT_VERSIONTUPLE_H