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

[Manman Ren via Phabricator via cfe-commits]

manmanren marked an inline comment as done.
manmanren added inline comments.


================
Comment at: include/clang/Serialization/Module.h:19
 #include "clang/Basic/FileManager.h"
+#include "clang/Basic/Module.h"
 #include "clang/Basic/SourceLocation.h"
----------------
rsmith wrote:
> Redundant include?
Definition of ASTFileSignature is moved from Serialization/Module.h to Basic/Module.h so we need to have this include to get the definition.


================
Comment at: lib/Serialization/ASTWriter.cpp:1356
+  ASTFileSignature RetCode{{0}};
+  if (WritingModule && Context.getLangOpts().ImplicitModules)
+    RetCode = WriteHash(0);
----------------
rsmith wrote:
> I don't think it makes sense to use `ImplicitModules` to control this. Doing that for the old signature computation was at best a hack.
> 
> If there is no measurable performance difference from the hashing, we should just do it unconditionally in all modes. Otherwise, there should be a separate flag to control it; we should probably force that flag to be enabled when the frontend implicitly builds a module and inserts it into the module cache, but otherwise let the user control it as they see fit.
Use HSOpts.ModulesHashContent to guard the hashing of module file content. This flag is set to true when we implicitly build a module.


https://reviews.llvm.org/D27689