[PATCH] D158573: [clang][modules] Move `UNHASHED_CONTROL_BLOCK` up in the AST file

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added inline comments.


================
Comment at: clang/lib/Serialization/ASTReader.cpp:2685
+  for (unsigned I = 0; I < ASTFileSignature::size; ++I)
+    Sig[I] = endian::readNext<uint8_t, little, aligned>(Blob);
+  return Sig;
----------------
uint8_t has no endianness and has alignment 1 anyway; you can just load it directly or use the same implementation as ASTFileSignature::create


================
Comment at: clang/lib/Serialization/ASTWriter.cpp:1143
 
-ASTFileSignature ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
-                                                      ASTContext &Context) {
+static constexpr ASTFileSignature DummySignature{
+    {1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}};
----------------
I wonder if we should hoist these dummy values to the ASTFileSignature type and then assert that they are never read from an ASTReader


================
Comment at: clang/lib/Serialization/ASTWriter.cpp:1152
+  using namespace llvm::support;
+  endian::Writer LE(Out, little);
+  for (uint8_t Byte : Sig)
----------------
We don't need an endian writer when it's bytes


================
Comment at: clang/lib/Serialization/ASTWriter.cpp:1161
+  if (WritingModule &&
+      PP->getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent) {
+
----------------
Nit: I think this would be clearer with an early-exit now that it's in a separate function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158573