[PATCH] D81347: Make ASTFileSignature an array of 20 uint8_t instead of 5 uint32_t

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.


================
Comment at: clang/include/clang/Basic/Module.h:29
 #include "llvm/ADT/iterator_range.h"
+#include <algorithm>
 #include <array>
----------------
aprantl wrote:
> Can you move the code that depends on algorithm into a .cpp file?
That code is the call to `std::copy` in the template function so it can't really be moved out... But I removed the include because `STLExtras.h` already includes it, so I think it is ok to remove the include.


================
Comment at: clang/include/clang/Basic/Module.h:70
+
+  static ASTFileSignature createDISentinel() {
+    ASTFileSignature Sentinel;
----------------
aprantl wrote:
> dang wrote:
> > I couldn't find anywhere in the code base that checks the bit-pattern so I left it as is. If anyone knows if someone could point me to the code that checks this (if any code does check it) that would be nice.
> The PCM hash is reused in CGDebugInfo as the DW_AT_dwo_id hash for the -gmodules debug info. This value is checked by dsymutil and LLDB.
DW_AT_dwo_id is 64 bit so using `0xFF` everywhere is fine.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81347/new/

https://reviews.llvm.org/D81347