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

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.


================
Comment at: include/clang/Serialization/ASTBitCodes.h:322
+
+      /// \brief Record code for the signature that identifiers this AST file.
+      SIGNATURE,
----------------
FYI, could be fixed in a separate commit for the entire file: These uses of \brief are redundant and we don't need them any more. We use the autobrief option in our Doxygen configuration file.


================
Comment at: lib/CodeGen/CGDebugInfo.cpp:1978
     // but LLVM detects skeleton CUs by looking for a non-zero DWO id.
-    uint64_t Signature = Mod.getSignature() ? Mod.getSignature() : ~1ULL;
+    // We use the lower 64 bits for debug info.
+    uint64_t Signature = Mod.getSignature() != ASTFileSignature({{0}}) ?
----------------
Just wanted to comment that this change LGTM.


================
Comment at: lib/CodeGen/ObjectFilePCHContainerOperations.cpp:247
+    uint64_t Signature = Buffer->Signature != ASTFileSignature({{0}}) ?
+        (uint64_t)Buffer->Signature[1] << 32 | Buffer->Signature[0] : ~1ULL;
     Builder->getModuleDebugInfo()->setDwoId(Signature);
----------------
Maybe factor this out into a getDwoId() helper function?


https://reviews.llvm.org/D27689