[PATCH] D27689: Module: hash the pcm content and use it as SIGNATURE for implicit modules.

[Richard Smith via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/include/clang/Serialization/ASTBitCodes.h:258
+      /// A block containing unhashed contents. It currently holds Diagnostic
+      /// Options and Signature.
+      UNHASHED_CONTROL_BLOCK_ID,
----------------
This comment is out of date. Maybe just point to the `UnhashedControlBlockRecordTypes` for the definitive list of records within this block?


================
Comment at: clang/lib/Serialization/ASTReader.cpp:2240-2241
 
+  // Lambda to read the unhashed control block the first time it's called.
+  bool HasReadUnhashedControlBlock = false;
+  auto readUnhashedControlBlockOnce = [&]() {
----------------
I guess the reason to do this is because reading that block depends on certain things in this block having been already read, and reading other things in this block depends on that block having been read? A comment explaining that would be useful.


================
Comment at: clang/lib/Serialization/ASTReader.cpp:4014-4015
+    case UNHASHED_CONTROL_BLOCK_ID:
+      // This block is handled using look-ahead during ReadControlBlock.  We
+      // shouldn't get here!
+      return Failure;
----------------
We shouldn't return `Failure` without producing an error message.


https://reviews.llvm.org/D27689